Skip to content

fix: keep staging Access login on the Shell ingress - #1092

Merged
BleedingDev merged 1 commit into
mainfrom
fix/stage-access-domains-20261004
Oct 4, 2026
Merged

BleedingDev merged 1 commit into
mainfrom
fix/stage-access-domains-20261004

Conversation

@BleedingDev

@BleedingDev BleedingDev commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Why the change

Keep staging login on the Shell domain so Cloudflare Access cannot redirect users to retired module domains that fail TLS.

Special things to note

  • Login and CI policies stay the same; immutable module releases keep their existing Workers URLs.
  • The live staging login destination is already repaired, with unchanged policies and token audience.
  • Regression coverage checks stale destination repair and provisioning without duplicate applications.

Change outline

 Cloudflare Access
-  Login → ten retired module domains → Shell
+  Login → ontos-stage.bleeding.dev

 Public bypass paths
-  Module federation assets and module contracts on retired domains
   Shell gateway context
   Shell runtime contract and federation manifest

Summary by CodeRabbit

  • Access Control
    • Staging access policies now cover the Shell hostname, gateway route and runtime metadata endpoints. Retired module hostnames and their endpoints are no longer included in these policies.
    • When staging access enforcement is disabled, the Shell ingress remains outside Access.
  • Reliability
    • If access applications are changed to use retired module hostnames, the drift is detected and provisioning restores the expected Shell destinations and policies.

@semanticdiff-com

semanticdiff-com Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  app/scripts/ops/cloudflare-stage-cost-guard.mts  67% smaller
  app/scripts/tests/cloudflare-stage-cost-guard.test.mts  35% smaller

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@BleedingDev
BleedingDev enabled auto-merge October 4, 2026 01:32
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (3)
app/README.md — auto-discovered
app/AGENTS.md — auto-discovered
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: eb314029-671a-4965-8e25-2228d51473e5
📥 Commits

Reviewing files that changed from the base of the PR and between 3215459 and 828359f.

📒 Files selected for processing (2)
  • app/scripts/ops/cloudflare-stage-cost-guard.mts
  • app/scripts/tests/cloudflare-stage-cost-guard.test.mts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (20)
  • GitHub Check: Build node artifacts (1/5)
  • GitHub Check: Build cloudflare artifacts (3/5)
  • GitHub Check: Typecheck, Effect Rule Implementation, and OntOS API Boundary Rules
  • GitHub Check: Build node artifacts (3/5)
  • GitHub Check: Build cloudflare artifacts (1/5)
  • GitHub Check: Build cloudflare artifacts (2/5)
  • GitHub Check: Unit and Component Tests (Catalog, Commerce Customer Context, and Inventory)
  • GitHub Check: Format and Lint through the pre-commit hook (2/2)
  • GitHub Check: Build cloudflare artifacts (4/5)
  • GitHub Check: Build cloudflare artifacts (5/5)
  • GitHub Check: Build node artifacts (4/5)
  • GitHub Check: Build node artifacts (5/5)
  • GitHub Check: Build node artifacts (2/5)
  • GitHub Check: Static Contracts
  • GitHub Check: Database, Migration, RLS, Authorization, and Outbox Integration
  • GitHub Check: Authenticated Browser Tests
  • GitHub Check: Unit and Component Tests (every other package)
  • GitHub Check: Format and Lint through the pre-commit hook (1/2)
  • GitHub Check: Repository Tooling, Module Entrypoint, Deployment Impact Planner, and Codesmith Generation Tests
  • GitHub Check: Quality Audit Guardrails
🧰 Additional context used
📓 Path-based instructions (3)
Source excerpt: Use Effect for application behavior, I/O, resource management, concurrency, dependencies, BFF contracts and clients, schemas, and expected failures.

📄 CodeRabbit inference engine (app/README.md)

Files:

  • app/scripts/tests/cloudflare-stage-cost-guard.test.mts
  • app/scripts/ops/cloudflare-stage-cost-guard.mts
Source excerpt: Before changing files under `app/`, read [the application coding guide](./README.md).

📄 CodeRabbit inference engine (app/AGENTS.md)

Files:

  • app/scripts/tests/cloudflare-stage-cost-guard.test.mts
  • app/scripts/ops/cloudflare-stage-cost-guard.mts
Source excerpt: Application work belongs under `app/` and follows [`app/AGENTS.md`](app/AGENTS.md).

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • app/scripts/tests/cloudflare-stage-cost-guard.test.mts
  • app/scripts/ops/cloudflare-stage-cost-guard.mts
🔇 Additional comments (2)
app/scripts/ops/cloudflare-stage-cost-guard.mts (1)

7-7: LGTM!

Also applies to: 42-44, 63-64, 89-89, 377-378, 383-405, 416-416, 421-421

app/scripts/tests/cloudflare-stage-cost-guard.test.mts (1)

155-155: LGTM!

Also applies to: 181-181, 186-191, 193-194, 239-272


Walkthrough

The stage cost guard now creates Access applications for Shell ingress and Shell contract paths. Its tests verify those destinations and confirm that provisioning restores the applications when their destinations drift.

Changes

Shell Access destinations

Layer / File(s) Summary
Define Shell Access destinations
app/scripts/ops/cloudflare-stage-cost-guard.mts
Access bypass applications now cover the Shell gateway-context route and Shell contract paths. The protected application covers only the Shell hostname. The disabled-enforcement message now refers to Shell ingress.
Verify destinations and drift repair
app/scripts/tests/cloudflare-stage-cost-guard.test.mts
Tests expect only Shell destinations. A new test changes all three applications to a retired module hostname, checks for drift, then verifies that provisioning restores the original applications with three PUT requests.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 82835

No identified issue blocks merging the Shell-only staging Access change after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 82835

Shell login policies and application identities are preserved, and regression coverage checks destination repair without duplicate applications during serial provisioning. However, removing login protection from ten module domains depends on their retirement. Those domains remain in deployment configuration, and their deployed reachability has not been established.

Retained concerns

  • Medium · security · inferred: Shell-only enforcement depends on an unverified retirement boundary. Deployment placement still declares the ten removed vertical domains and their units. If any domain remains or becomes routed to a service, this migration removes its former Cloudflare login gate, not merely its artifact bypass entries. Immutable artifact URLs do not establish that those domains have no service traffic; deployed reachability and downstream authorization remain unresolved.
Security review details

Security Blast Radius

  • inferred — The changed enforcement scope is staging Shell plus ten removed vertical domains. The placed Worker model includes private data-plane bindings, so these units cannot be assumed to be static artifact hosts. Direct datastore access, tenant-wide compromise and production exposure are not established by the reviewed evidence.

Security Findings and Attack Paths

  • inferred — If a removed vertical domain serves a live application, an Internet caller could reach it without the Cloudflare login or CI token previously required outside bypass paths. Whether application-level authorization prevents sensitive outcomes is unresolved. This is a conditional control-removal concern, not a verified sensitive-operation bypass.

Trust Boundaries and Controls

  • observed — With enforcement enabled, the protected Shell application retains people and CI policy references with declared precedence 1 and 2. Gateway-context and contract paths use the separate bypass policy. Existing application IDs are preserved during fixture-based destination repair; external audience preservation is not asserted.

Resilience and Maintainability Implications

  • observed — Disabling enforcement returns without removing previously provisioned applications, and application creation follows a list/find/create sequence without serialization in the inspected flow. These lifecycle limitations existed in the base; serial idempotency tests do not prove concurrent uniqueness or an enforcement-off rollback.

Hardening Proposals

  • proposed — Bind removal of domain-level protection to explicit retirement evidence or a documented replacement authorization boundary, and keep deployment placement consistent with that decision.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: keeping staging Access login on the Shell ingress.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@BleedingDev
BleedingDev added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit 035e1ea Oct 4, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant