Skip to content

Enforce template lookup scope after compatible rollout - #806

Open
nickdujay-pplx wants to merge 3 commits into
mainfrom
codex/template-lookup-enforcement
Open

nickdujay-pplx wants to merge 3 commits into
mainfrom
codex/template-lookup-enforcement

Conversation

@nickdujay-pplx

@nickdujay-pplx nickdujay-pplx commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Strict template lookup checks previously stopped valid requests during mixed-version upgrades (#734). Operators need to install support across services before activating those checks.

This restores the authorization behavior previously published in #217, with enforcement off by default. Merging and deploying this PR does not activate enforcement. The compatibility stage, #736, is merged.

The authorization rules and original evidence are documented in #217. This PR separates protocol installation from operator-controlled activation. Compatibility receipts and authorization acknowledgements remain separate, and trusted local execution retains offline cache access.

Direct creation records its target before dispatch so the worker can authorize the lookup. The two guarded placement writes stay on the existing Direct path because the placement machine owns Kubernetes only; the architecture baseline change makes that exception explicit. No schema migration is required.

Validation

  • Current-head formatting, affected-package all-target/all-feature Clippy, and architecture checks pass.
  • 161 targeted request, catalog/PostgreSQL, placement, and compatibility cases pass with required database fixtures enabled.
  • Real node-api and template-manager binaries pass 33 request scenarios on a Linux devbox: compatible and enforcement-capable clients, enforcement off/on, allowed and denied catalog responses, missing/wrong authorization acknowledgements, local offline access, and unauthenticated request rejection.
  • The binary replay uses a synthetic catalog and cached artifact. Real catalog/PostgreSQL authorization is covered separately by request tests. This does not prove a completed mixed-version VM create, restore, or template build. Those are required before activation below. Task processes and fixtures were cleaned up.
  • The catalog request test covers reviewed Kubernetes node identity with both legacy and required-scope requests.
  • The teardown registration test keeps its background uploader idle so the registration verdict does not depend on which task finishes first. Its generation and refusal assertions are unchanged; 20 consecutive devbox runs pass. The earlier CI failure did not reproduce in 20 local runs before this change.
  • Fresh CI is running for this head. All four Kind/Firecracker shards passed on the previous head; that run failed one teardown registration test, addressed above. Ordinary Kind coverage does not establish enforcement-on coverage.

Severity and risk

Severity: high availability impact if activated before the rollout prerequisites are met. The main risk is availability: enabling enforcement while old catalog replicas remain makes otherwise valid remote lookups fail. Enforced cache reads also depend on catalog availability. Existing placed sandboxes retain access to retiring templates.

Rollout

  1. Deploy the compatibility stage from Prepare template lookup scope for rolling upgrades #736 across participating services.
  2. Install this version with SPACE_TEMPLATE_LOOKUP_ENFORCEMENT unset or false. Verify every catalog supports required-scope requests and every API records placement before Direct dispatch. Compatibility receipts alone do not prove either condition.
  3. Enable enforcement progressively on template-managers. Before broad activation, verify allowed and denied cache hits/misses and complete authorized create, restore, and build workflows.

The flag is read at startup; invalid values fail startup. Disabling it or rolling template-manager back restores legacy access and removes enforcement. Every serving template-manager must have enforcement enabled for activation to be complete.

AI assistance: OpenAI Codex contributed the implementation, tests, and description.

@nickdujay-pplx
nickdujay-pplx force-pushed the codex/template-lookup-enforcement branch from 1926b46 to 35bf58b Compare October 21, 2026 21:04
@nickdujay-pplx
nickdujay-pplx force-pushed the codex/template-lookup-compatible-rollout branch from 0b214e2 to 029b658 Compare October 22, 2026 13:33
@nickdujay-pplx
nickdujay-pplx force-pushed the codex/template-lookup-enforcement branch from 35bf58b to 388cab6 Compare October 22, 2026 13:35
Base automatically changed from codex/template-lookup-compatible-rollout to main October 22, 2026 14:33
@nickdujay-pplx
nickdujay-pplx force-pushed the codex/template-lookup-enforcement branch from 388cab6 to 0fc3931 Compare October 22, 2026 15:26
@nickdujay-pplx
nickdujay-pplx marked this pull request as ready for review October 22, 2026 15:33
Keep installation compatible until operators enable catalog authorization.
Require distinct authorization acknowledgements and validate delegated scope
before remote cache reads, while preserving trusted local execution.
@nickdujay-pplx
nickdujay-pplx force-pushed the codex/template-lookup-enforcement branch from 0fc3931 to 850bdcd Compare October 22, 2026 23:28
@cla-assistant

cla-assistant Bot commented Oct 22, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@shrikant-pplx shrikant-pplx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current PR head. Approving the default-off rollout, with the P2 Direct restore finding in the inline comment to address before enabling enforcement.

The functional review and independent security review are complete. No confirmed security findings. CI for this commit passed, including all four Kind/Firecracker shards; ordinary Kind coverage does not establish enforcement-on coverage. This review used code inspection and existing validation evidence, without additional runtime execution.

  1. Does the change precisely and completely fix the reported problem and its root cause? Partly — the default-off gate separates installation from activation, but Direct restore does not record its destination before the new placement authorization check.
  2. Does it cover every affected caller and execution path? No — enforcement-on Direct cross-node restore with node-scoped catalog authentication remains affected.
  3. Is every change necessary, with no unrelated behavior or scope creep? Yes — the changes support staged enforcement, placement authorization, and the stated validation.
  4. Do the tests reproduce the original failure and prove the corrected behavior and important edge cases? Partly — component and compatibility coverage exists, but the Direct restore authorization ordering is not exercised end to end.
  5. Does the change introduce a correctness, security, performance, compatibility, or operational regression under realistic conditions? Yes — the restore denial described in the inline comment occurs with enforcement enabled; default-off deployment avoids that regression until activation.

Comment on lines +233 to +237
&& !sandbox
.active_node_id
.iter()
.chain(sandbox.target_node_id.iter())
.any(|id| node_ids.contains(id))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 [func] Record the Direct restore destination before template authorization — crates/libs/internal/api/server/src/adapter.rs:233

With enforcement enabled and catalog authentication scoped to registered nodes, a Direct cross-node restore of a platform-owned sandbox from a btrfs-send snapshot can fail here. stage_btrfs_send_restore calls ensure_local_template even when the base is cached, and that request uses the sandbox scope. However, restore_execution_with_candidates dispatches Start before recording the selected destination in active_node_id or target_node_id; persist_owned_restore_route_identity_receipt updates those pointers only after Start succeeds. The destination therefore receives PermissionDenied before it can finish restoring and establish the placement this check requires. This also affects Direct nodes when TokenReview is configured with static fallback, since those callers receive node-scoped authorization. Record the selected destination under the existing restore lease before dispatch, preserving source placement and safely releasing or replacing failed candidate claims, and extend the Direct restore regression coverage to exercise this lookup with enforcement enabled.

This branch has not been deployed

No deployments
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.

2 participants