fix(helm): improve spiffe id configuration in ci overlay by grs · Pull Request #3100 · NVIDIA/OpenShell

grs · 2026-09-01T17:56:08Z

Summary

Updates the local SPIRE dev/ci overlay so sandbox JWT-SVID subjects include namespace, pod name, and the OpenShell sandbox ID as one opaque final SPIFFE ID segment and will apply to any namespace in which there are OpenShell managed pods. This makes it work whatever workspace mapping mode is in use. It is also more protected against spoofing.
Note: neither the gateway nor the supervisor have any specific requirements for the form of the SPIFFE ID.

Related Issue

Fixes #3037

Changes

updates the spire stack values to change the spiffe id configuration
updates docs
updates text in token grant and token exchange demos
adds some fixes to the token grant demo to allow https on the openshell gateway and in retrieval of the spire oidc keys

Testing

mise run pre-commit passes
existing examples work with change
Unit tests added/updated
E2E tests added/updated (if applicable)

Checklist

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

Signed-off-by: Gordon Sim <[email protected]>

copy-pr-bot · 2026-09-01T17:56:12Z

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.

github-actions · 2026-09-16T08:33:54Z

This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity.

grs · 2026-09-16T10:40:45Z

@mrunalp Could you have a quick glance at this when you have a moment? It is very small, promise!

mrunalp · 2026-09-24T21:10:05Z

Recommendation: request changes on the SPIFFE identity scope.

Before this PR, the overlay issued sandbox shaped SVIDs only to pods in the openshell namespace. The new empty namespaceSelector (

OpenShell/deploy/helm/openshell/ci/values-spire-stack.yaml

Lines 25 to 37 in ae896d7

    
           spiffeIDTemplate: 'spiffe://{{ .TrustDomain }}/openshell/sandbox/{{ .PodMeta.Namespace }}.{{ .PodMeta.Name }}{{ with (index .PodMeta.Annotations "openshell.ai/sandbox-id") }}.{{ . }}{{ end }}' 
        
           # Match sandbox pods in shared, managed, and operator workspace modes. 
        
           # This CI/dev overlay intentionally leaves namespaceSelector broad so it 
        
           # works without requiring namespace labels. To restrict identity issuance 
        
           # to OpenShell-managed namespaces, label those namespaces and uncomment 
        
           # the namespaceSelector block below. 
        
           namespaceSelector: {} 
        
           # namespaceSelector: 
        
           #   matchLabels: 
        
           #     openshell.ai/managed-by: openshell 
        
           podSelector: 
        
             matchLabels: 
        
               openshell.ai/managed-by: openshell

) matches
every namespace watched by SPIRE; the remaining pod label can be set by anyone permitted to create a pod there. The sandbox ID annotation is now optional. In the demo, the token issuer accepts any signed sandbox-prefix subject
(

OpenShell/examples/spiffe-token-grant-demo/k8s/token-issuer.js

Lines 131 to 140 in ae896d7

    
           if (parsed.payload.iss !== SPIRE_ISSUER) { 
        
             throw new Error(`unexpected JWT-SVID issuer ${parsed.payload.iss}`); 
        
           } 
        
           if (!hasAudience(parsed.payload, JWT_SVID_AUDIENCE)) { 
        
             throw new Error(`JWT-SVID audience did not include ${JWT_SVID_AUDIENCE}`); 
        
           } 
        
           if (!String(parsed.payload.sub || "").startsWith(TRUST_DOMAIN_PREFIX)) { 
        
             throw new Error("JWT-SVID subject was not an OpenShell sandbox SPIFFE ID"); 
        
           } 
        
           return parsed.payload;

), so an unrelated pod could obtain an SVID and exchange it for demo access tokens. Kubernetes confirms that an empty label selector
matches all objects (https://kubernetes.io/docs/reference/kubernetes-api/definitions/label-selector-v1-meta/). I’d use the managed namespace label as the selector and ensure Shared and Operator namespaces receive it; that also satisfies issue #3037’s acceptance criterion
(#3037).

grs · 2026-09-24T21:15:44Z

@mrunalp that is addressed in the comments in the config: https://github.com/NVIDIA/OpenShell/pull/3100/changes#diff-4b34dc99efb10c67663e2f32dca23f2ee36e1b95b0ec68ac3bb85dc265f0729cR26-R34, the selector still matches on pod label, so it is opt in.

A SPIFFE ID built only from the openshell.ai/sandbox-id annotation is spoofable: any pod that copies the annotation and labels gets the same ID and matches the sandbox's Keycloak client. Use <namespace>.<pod-name>.<sandbox-id>, the form Gordon Sim proposed upstream in NVIDIA/OpenShell#3100. - 02-sandbox-spiffe-ids.sh: ClusterSPIFFEID uses the #3100 template; our namespace and supervisor-pod selectors stay - keycloak-registrar: {namespace} (SANDBOX_NAMESPACE) and {podName} (os-supervisor-<lowercased id>, as the Kubernetes driver names it) placeholders; new default template; manifest sets SANDBOX_NAMESPACE - Docs and try-it output updated; credit to Gordon Sim - workarounds.md: upstream links for group-to-workspace mapping Tested: 38 unit tests; on OpenShift the token-exchange demo returns azp=.../openshell-odh.os-supervisor-<id>.<id>; a pod copying the annotation gets .../openshell-odh.spoof-test.<id> with no Keycloak client. Co-Authored-By: Claude Opus 5.5 <[email protected]>

github-actions · 2026-10-09T08:37:44Z

This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity.

grs · 2026-10-09T09:32:04Z

@mrunalp anything else on this that is needed?

github-actions · 2026-10-25T08:29:54Z

This pull request has had no activity for 14 days and is now marked stale.

TaylorMutch · 2026-11-10T23:54:04Z

@grs, is this change still needed on current main, and are you available to follow up?

Your reply explains the intended pod-label opt-in, but Mrunal's concern appears to remain outstanding: with namespaceSelector: {}, someone permitted to create pods in another watched namespace can also supply the matching pod label. The commented-out namespace restriction does not enforce an administrator-controlled boundary.

If you're continuing this work, please address that concern by restricting issuance to administrator-approved namespaces and covering Shared, Managed, and Operator modes, as Mrunal suggested, or provide evidence of the admission/RBAC controls that make the broader selector appropriate for this overlay. Please also confirm the token-grant demo's subject-prefix acceptance is appropriate for the resulting issuance scope so we can continue the acceptance review.

grs · 2026-11-11T16:31:52Z

@TaylorMutch The ci/dev SPIRE config still proposes a trivially spoofable pattern for IDs and is still applied only to a namespace named openshell.

With regard to the namespaceSelector, if it is set and only matches namespaces with 'openshell.ai/managed-by: openshell', then in operator- or shared- mode it would not apply unless that label is explicitly added to the namespace. If the specific pattern is not applied then the default pattern will be used, which is based on service account. In that case all sandboxes in the same namespace would have the same identity.

If on the other hand the selector is left empty and a rogue pod opts in to the pattern, they get an ID specific to their pod. They cannot spoof an existing sandbox. The fact that it matches the pattern chosen for sandboxes should not itself be a security risk.

So in my view, it is 'safer' to match all namespaces by default but clearly indicate how this can be made more specific in comments. All that said however, if there is still a clear preference for the commented out version of the selector, that is a simple change to make and I can make it.

With regard to the example, that uses a dummy token service whose only check on the subject is a prefix check. That is the case independent of the change proposed in this PR. It is offered as a simplified way to see the grant pattern, and not intended as a real token service for actual use. The only impact on the example that the change to the SPIFFE ID pattern has is that the example would accept JWT-SVIDs from any pod on the cluster, rather than just those in namespace 'openshell'. The token issued will however match the actual pod that requested it and even for the example is not spooffable with the change on this PR applied. So in my view the example is 'safer' overall with these changes.

grs added 2 commits September 1, 2026 15:10

fix(helm): improve spiffe id configuration in ci overlay

4be070a

Signed-off-by: Gordon Sim <[email protected]>

fix(examples): improvements to spiffe-token-grant-demo

ae896d7

Signed-off-by: Gordon Sim <[email protected]>

grs requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 1, 2026 17:56

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Sep 16, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Sep 17, 2026

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Oct 9, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Oct 10, 2026

github-actions Bot added the state:stale Inactive item at risk of automatic closure. label Oct 25, 2026

github-actions Bot removed the state:stale Inactive item at risk of automatic closure. label Nov 11, 2026

Navigation Menu

Repository navigation

fix(helm): improve spiffe id configuration in ci overlay - #3100

fix(helm): improve spiffe id configuration in ci overlay#3100
grs wants to merge 2 commits into
NVIDIA:mainfrom
grs:spiffe-id-configuration

grs commented Sep 1, 2026

Uh oh!

copy-pr-bot Bot commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

grs commented Sep 16, 2026

Uh oh!

mrunalp commented Sep 24, 2026

Uh oh!

grs commented Sep 24, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

grs commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

TaylorMutch commented Nov 10, 2026

Uh oh!

grs commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

grs commented Sep 1, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

grs commented Sep 16, 2026

Uh oh!

mrunalp commented Sep 24, 2026

Uh oh!

grs commented Sep 24, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

grs commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 25, 2026

Uh oh!

TaylorMutch commented Nov 10, 2026

Uh oh!

grs commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants