Skip to content

fix(isolation): enforce workload identity invariants on deserialize - #5192

Merged
FrostGod merged 1 commit into
NVIDIA:mainfrom
EmilienM:fix/5189-identity-deser-invariants/EmilienM
Nov 6, 2026
Merged

FrostGod merged 1 commit into
NVIDIA:mainfrom
EmilienM:fix/5189-identity-deser-invariants/EmilienM

Conversation

@EmilienM

@EmilienM EmilienM commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Summary

ResolvedWorkloadIdentity derived Deserialize with public fields, so a value decoded from a serialized descriptor skipped the checks new() enforces. Deserialization now goes through a private wire mirror and TryFrom, which delegates to new(): UID/GID 0 and empty or whitespace-only source/digest fail to decode, and supplementary groups are sorted, de-duplicated, and stripped of the primary GID during decode. An unsorted group list in a descriptor used to fail the later match against the admitted identity; it now decodes to the canonical form, so the same logical identities are accepted as before. The wire format for valid values is unchanged.

Related Issue

Closes #5189

Changes

  • Added RawResolvedWorkloadIdentity (private, deny_unknown_fields) and #[serde(try_from)] on ResolvedWorkloadIdentity, delegating to new()
  • Deserialization tests: UID/GID/supplementary 0, empty and whitespace-only source and digest, group sorting, dedup, and primary-GID removal on decode, round-trip, and rejection of an invalid identity embedded in BoundaryConfirmation
  • Follow-ups from the same code read, for types whose validate() covers only part of what they carry:
    • BoundaryConfirmation: session ID, resource claims, and evidence digest are checked by the RPC client, not on decode
    • ProvisioningEvidence: an empty generation is only caught by validate(), and the digest binding cannot be re-verified at decode
    • ExecutableIdentity/BinaryIdentity: absolute-path and ancestor shape are unvalidated on decode

Release Notes

Descriptors carrying an invalid workload identity (UID or GID 0, empty source or resource digest) now fail to deserialize instead of being accepted.

Testing

  • Checks appropriate to the affected code and behavior pass (cargo test for openshell-isolation-interface, openshell-sandbox-backend, openshell-supervisor, openshell-driver-{docker,podman,kubernetes,vm}, openshell-sandbox; rust:lint)
  • Unit tests added/updated
  • E2E tests added/updated (not applicable: no runtime behavior change)

Checklist

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

@copy-pr-bot

copy-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@FrostGod

FrostGod commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 2057e57

@EmilienM
EmilienM force-pushed the fix/5189-identity-deser-invariants/EmilienM branch from 2057e57 to d0d47c3 Compare November 4, 2026 03:26
@mrunalp mrunalp added the area:sandbox Sandbox runtime and isolation work label Nov 4, 2026
@mrunalp

mrunalp commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

/ok to test d0d47c3

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

One non-blocking test coverage suggestion.

ResolvedWorkloadIdentity derives Deserialize with public fields, so a value
decoded from a serialized descriptor (for example the supervisor's
SandboxRuntimeDescriptor payload) bypassed the non-root, non-empty, and
sorted-unique-group invariants that new() enforces. Route deserialization
through a private raw mirror type and a TryFrom impl that delegates to
new(), so invalid identities fail to decode and supplementary groups are
normalized on the way in. The wire format for valid values is unchanged.

Signed-off-by: Emilien Macchi <emacchi@redhat.com>
@EmilienM
EmilienM force-pushed the fix/5189-identity-deser-invariants/EmilienM branch from d0d47c3 to abf2487 Compare November 5, 2026 14:15
@FrostGod

FrostGod commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

/ok to test abf2487

@FrostGod
FrostGod enabled auto-merge November 6, 2026 17:55
@FrostGod
FrostGod added this pull request to the merge queue Nov 6, 2026
Merged via the queue into NVIDIA:main with commit cc4acdc Nov 6, 2026
83 of 84 checks passed
@FrostGod
FrostGod deleted the fix/5189-identity-deser-invariants/EmilienM branch November 6, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:sandbox Sandbox runtime and isolation work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: ResolvedWorkloadIdentity deserialization skips the non-root and ordering invariants of new()

3 participants