Skip to content

fix(sandbox): check every backend confirmation against the launch - #4706

Open
shiju-nv wants to merge 2 commits into
NVIDIA:mainfrom
shiju-nv:fix/4703-reconfirm-launch-binding/shiju-nv
Open

shiju-nv wants to merge 2 commits into
NVIDIA:mainfrom
shiju-nv:fix/4703-reconfirm-launch-binding/shiju-nv

Conversation

@shiju-nv

@shiju-nv shiju-nv commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The delegated client compared a backend's Confirmed with the launch only on the first confirmation. Credential renewal, the liveness probe and reconnect accepted any Confirmed, and reconnect ignored the replayed Attach reply. This PR checks every Attach and Confirm reply against the launch and documents that both must stay replayable for the whole generation.

Related Issue

Closes #4703

Changes

  • crates/openshell-sandbox-backend/src/runtime.rs:
    • BoundaryClient::check_attached and BoundaryClient::launch_confirmation check a reply against the client's runtime descriptor (workload identity, generation, session, resource claims, provisioning evidence). A wrong response variant is rejected, and a mismatch error names the field.
    • First attach and confirm use them, and so do the three replay paths:
      • ensure_current_credential_connection: credential renewal;
      • connection_is_live: the liveness probe;
      • attach_new_channel: reconnect.
    • On a same-connection renewal that returns another launch, the connection is closed and the mediation session cleared.
    • BoundBoundary loses its copies of the launch fields, since the client's descriptor is now the single source.
  • proto/sandbox_protocol.proto: Attach and Confirm replays succeed for the whole generation; the supervisor replays them on reconnect, and Confirm alone for renewal and liveness. The client checks every reply.
  • Tests:
    • TestGrpcBoundary gains attach_generation.
    • New tests in runtime/tests/credential_renewal.rs:
      • renewal and reconnect each reject a confirmation whose identity, generation (including empty), session, resource claims or provisioning evidence differs; renewal also closes the connection. The cases come from one shared table, and each checks the error names the field;
      • check_attached and launch_confirmation reject the wrong response variant;
      • reconnect whose Attach reports another generation is rejected.

Testing

  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@TaylorMutch TaylorMutch 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.

The centralized validation is the right shape and closes the gap across first confirmation, renewal, liveness, and reconnect. Please address these items before merge:

  1. Update the branch onto current main and rerun CI. This head is 30 commits behind, and crates/openshell-sandbox-backend/src/runtime.rs changed in the intervening transport work (#4656).
  2. Complete the replay-path test coverage described by #4703. The new tests cover a session mismatch during renewal/reconnect and an Attach generation mismatch. Please also cover resource-claim and provisioning-evidence mismatches across renewal/reconnect, plus wrong response variants for replayed Attach and Confirm. A table-driven validator test can keep this compact, with path-level tests proving the shared validator is called.
  3. Keep mismatch diagnostics field-specific. launch_confirmation calls BoundaryConfirmation::validate before its field checks, so an identity mismatch or empty generation currently returns the generic boundary confirmation is incomplete or mismatched error. Please make those errors identify the mismatched field, matching the issue's acceptance criterion and the PR description.

Once those are addressed and CI is green on the updated base, this should be ready for another review.

The delegated client compared a backend's Confirmed with the launch
binding only on the first confirmation. Credential renewal, the
liveness probe and reconnect accepted any Confirmed, and reconnect
ignored the replayed Attach reply. A backend that lost its launch state
could answer for another session and keep serving the sandbox.

Check Attach and Confirm replies against the client's runtime
descriptor on every path. Close the connection when a same-epoch
renewal returns another launch. Document that Attach and Confirm must
stay replayable for the whole generation.

Closes NVIDIA#4703

Signed-off-by: Shiju <shiju@nvidia.com>
@shiju-nv
shiju-nv force-pushed the fix/4703-reconfirm-launch-binding/shiju-nv branch from 490275a to 826a94f Compare October 21, 2026 06:35
Attach and Confirm replays must return the original result for the whole
generation. The renewal and reconnect tests now match the full mismatch
phrase, so the generation case cannot pass on the empty-generation error.

Signed-off-by: Shiju <shiju@nvidia.com>

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.

bug(sandbox): reconnect, credential renewal and liveness accept a backend Confirm without checking the launch binding

2 participants