fix(router): enforce session-to-route binding validation by KinshukSS2 · Pull Request #324 · volcano-sh/agentcube

KinshukSS2 · 2026-05-11T18:55:11Z

Problem

When a client sends x-agentcube-session-id, the router retrieves the sandbox from the store and forwards the request without checking whether the session belongs to the namespace/name/kind in the URL. A valid session from Runtime A can be replayed against Route B, bypassing tenant and workload isolation.

Solution

Added strict three-field validation (namespace, name, kind) between the route and the session metadata stored at creation time. Mismatched sessions are rejected before forwarding with a 409 Conflict response and a SESSION_TARGET_MISMATCH error code.

Changes

Package	Change
`pkg/common/types`	Added `WorkloadKind`/`WorkloadName` to `SandboxInfo` to persist route-identity at session creation
`pkg/api/errors`	Added `NewSessionTargetMismatchError` (409) and `IsSessionTargetMismatch` predicate
`pkg/router/session_manager`	Rewrote `sessionTargetMatches`: fail-closed on empty metadata, strict 3-field comparison, `klog.Warning` on mismatch
`pkg/router/handlers`	Surfaces `SESSION_TARGET_MISMATCH` code in JSON response body
`pkg/workloadmanager`	Propagates `WorkloadKind`/`WorkloadName` through `sandboxEntry` → `buildSandboxPlaceHolder`/`buildSandboxInfo`

Testing

Unit tests cover: mismatch on each field, fail-closed on empty metadata, happy-path sticky-session reuse, SESSION_TARGET_MISMATCH in JSON body, field propagation through workload manager builders.
Pre-existing gocyclo lint violation fixed by extracting agentRuntimeCreateHandler helper.

ok  pkg/router          6.04s
ok  pkg/api             0.00s
ok  pkg/common/types    0.00s
ok  pkg/workloadmanager 0.18s
golangci-lint run ./...   # exits 0

volcano-sh-bot · 2026-05-11T18:55:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yaozengzeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

pkg/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gemini-code-assist

Code Review

This pull request implements session target validation to prevent incorrect session reuse across different workloads. It adds WorkloadKind and WorkloadName metadata to the sandbox information and introduces a check to ensure the retrieved sandbox matches the requested target. Critical feedback was provided regarding the createSandbox implementation, where overwriting the ephemeral sandbox name and resource kind causes observability regressions and breaks JWT authentication. Additionally, it is recommended to use warning-level logging for session mismatches to improve security visibility.

gemini-code-assist · 2026-05-11T18:57:14Z

+		klog.V(2).InfoS("Session target metadata missing", "sessionID", sessionID, "requestedNamespace", namespace, "requestedName", name, "requestedKind", kind, "missingFields", strings.Join(missingFields, ","))
+		return false
+	}
+	if sandbox.WorkloadKind != kind || sandbox.WorkloadName != name || sandbox.SandboxNamespace != namespace {
+		klog.V(2).InfoS("Session target mismatch", "sessionID", sessionID, "requestedNamespace", namespace, "requestedName", name, "requestedKind", kind, "actualNamespace", sandbox.SandboxNamespace, "actualWorkloadName", sandbox.WorkloadName, "actualWorkloadKind", sandbox.WorkloadKind)


The logging level for session mismatches and missing metadata should be Warning as stated in the PR description. Using V(2) Info logs makes these security-relevant events invisible in standard production environments. Consider using klog.Warningf to ensure these are captured.

Copilot

Pull request overview

This PR hardens the Router session reuse flow by binding a session ID to the specific route identity (namespace/name/kind) it was originally created for, preventing replay of a valid session against a different workload route.

Changes:

Persisted route identity on sessions by adding WorkloadKind/WorkloadName to types.SandboxInfo and populating them from Workload Manager.
Enforced strict session-to-route validation in SessionManager.GetSandboxBySession, returning a 409 Conflict on mismatch.
Added a dedicated API error/predicate for mismatch and surfaced SESSION_TARGET_MISMATCH in Router JSON error responses; expanded unit tests accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/workloadmanager/workload_builder.go	Populates workload identity fields on `sandboxEntry` during sandbox creation.
pkg/workloadmanager/k8s_client.go	Extends `sandboxEntry` to carry workload identity for later propagation.
pkg/workloadmanager/sandbox_helper.go	Persists `WorkloadKind`/`WorkloadName` into stored `SandboxInfo` for session reuse validation.
pkg/workloadmanager/sandbox_helper_test.go	Updates tests to assert workload identity propagation.
pkg/common/types/sandbox.go	Adds `WorkloadKind`/`WorkloadName` fields to the persisted session/sandbox metadata model.
pkg/router/session_manager.go	Adds strict session target validation and constructs session metadata on create path.
pkg/router/session_manager_test.go	Adds mismatch/match/fail-closed unit tests; refactors mock handler to reduce complexity.
pkg/api/errors.go	Adds mismatch conflict error constructor and predicate helper.
pkg/api/errors_test.go	Adds coverage for the new mismatch predicate.
pkg/router/handlers.go	Emits `SESSION_TARGET_MISMATCH` error code in JSON responses for the new conflict type.
pkg/router/handlers_test.go	Asserts `SESSION_TARGET_MISMATCH` is present in the JSON response body.

KinshukSS2 · 2026-05-11T19:01:19Z

+	// Construct Sandbox Info from response, storing the requested target metadata
+	// to enable proper session reuse validation. The ephemeral sandbox name (res.SandboxName)
+	// is not stored here to avoid the "fail-open" vulnerability where a session could be
+	// incorrectly reused for a different workload.
 	sandbox := &types.SandboxInfo{
-		SandboxID:   res.SandboxID,
-		Name:        res.SandboxName,
-		SessionID:   res.SessionID,
-		EntryPoints: res.EntryPoints,
+		SandboxID:        res.SandboxID,
+        Name:             res.SandboxName,
+        SandboxNamespace: namespace,
+        Kind:             types.SandboxKind,
+        WorkloadKind:     kind,
+        WorkloadName:     name,
+		SessionID:        res.SessionID,
+		EntryPoints:      res.EntryPoints,


@copilot apply changes based on this feedback

KinshukSS2 · 2026-05-11T19:01:23Z

+	if len(missingFields) > 0 {
+		klog.V(2).InfoS("Session target metadata missing", "sessionID", sessionID, "requestedNamespace", namespace, "requestedName", name, "requestedKind", kind, "missingFields", strings.Join(missingFields, ","))
+		return false
+	}
+	if sandbox.WorkloadKind != kind || sandbox.WorkloadName != name || sandbox.SandboxNamespace != namespace {
+		klog.V(2).InfoS("Session target mismatch", "sessionID", sessionID, "requestedNamespace", namespace, "requestedName", name, "requestedKind", kind, "actualNamespace", sandbox.SandboxNamespace, "actualWorkloadName", sandbox.WorkloadName, "actualWorkloadKind", sandbox.WorkloadKind)
+		return false


@copilot apply changes based on this feedback

KinshukSS2 · 2026-05-11T19:01:28Z

+func IsSessionTargetMismatch(err error) bool {
+	if !apierrors.IsConflict(err) {
+		return false
+	}
+	statusErr, ok := err.(apierrors.APIStatus)
+	if !ok {
+		return false
+	}
+	status := statusErr.Status()
+	if status.Details == nil {
+		return false
+	}
+	if status.Details.Group != sessionResource.Group || status.Details.Kind != sessionResource.Resource {
+		return false
+	}
+	return strings.Contains(status.Message, "session target mismatch")
+}


@copilot apply changes based on this feedback

Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

- Fix fail-open vulnerability: store requested metadata (namespace, name, kind) in SandboxInfo instead of ephemeral sandbox name for proper session reuse validation - Fix ephemeral name mismatch: sessionTargetMatches now compares against stored requested metadata instead of generated sandbox names - Add structured logging: log session target mismatches at V(2) level for better observability - Add comprehensive table-driven tests: cover perfect matches, mismatches, nil sandboxes, empty fields (wildcard matching), and edge cases Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

Reject reused session IDs that don't match the requested namespace/name/kind route tuple before forwarding. Changes: - pkg/common/types: add WorkloadKind and WorkloadName fields to SandboxInfo to persist the route-identity of each session at creation time, enabling deterministic comparison on reuse. - pkg/api/errors: add IsSessionTargetMismatch helper and NewSessionTargetMismatchError (409 Conflict) used by the session manager on mismatch. - pkg/router/session_manager: replace wildcard-permissive matching with strict three-field comparison (WorkloadKind, WorkloadName, SandboxNamespace). Fail-closed when any field is empty. Emit structured klog warning on mismatch. - pkg/router/handlers: surface SESSION_TARGET_MISMATCH error code in the JSON response body via IsSessionTargetMismatch check. - pkg/workloadmanager: propagate WorkloadKind/WorkloadName through sandboxEntry -> buildSandboxPlaceHolder/buildSandboxInfo so that sandboxes created by the workload manager carry the metadata needed for reuse validation. Resolves: session reuse isolation bug where a valid session from Runtime A could be replayed against Route B, bypassing tenant isolation and causing cross-runtime execution. Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

- handlers_test: add expectedBodyCode assertion to TestHandleInvoke_ErrorPaths/session-target-mismatch to verify the SESSION_TARGET_MISMATCH code is present in the JSON response. - session_manager_test: migrate all sessionTargetMatches fixtures to use WorkloadKind/WorkloadName fields; fix TestGetSandboxBySession_ TargetMatch_EmptyFields to assert conflict error (fail-closed); extract agentRuntimeCreateHandler helper to bring TestGetSandboxBySession_CreateSandbox_AgentRuntime_Success cyclomatic complexity below gocyclo threshold (was 17, now <=15). - sandbox_helper_test: populate WorkloadKind/WorkloadName in sandboxEntry fixtures and assert they are propagated to SandboxInfo. Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

Security-relevant events (missing metadata, session target mismatch) must be visible in production logs at all times. Promote both log lines from klog.V(2).InfoS (verbose/debug) to klog.Warningf so they are captured without enabling verbose logging. Addresses code review feedback from gemini-code-assist. Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

KinshukSS2 · 2026-05-11T19:19:18Z

+	// Construct Sandbox Info from response, storing the requested target metadata
+	// to enable proper session reuse validation. The ephemeral sandbox name (res.SandboxName)
+	// is not stored here to avoid the "fail-open" vulnerability where a session could be
+	// incorrectly reused for a different workload.
 	sandbox := &types.SandboxInfo{
-		SandboxID:   res.SandboxID,
-		Name:        res.SandboxName,
-		SessionID:   res.SessionID,
-		EntryPoints: res.EntryPoints,
+		SandboxID:        res.SandboxID,
+		Name:             name,
+		SandboxNamespace: namespace,
+		Kind:             kind,
+		WorkloadKind:     kind,
+		WorkloadName:     name,
+		SessionID:        res.SessionID,
+		EntryPoints:      res.EntryPoints,


@copilot apply changes based on this feedback

KinshukSS2 · 2026-05-11T19:19:11Z

+func IsSessionTargetMismatch(err error) bool {
+	if !apierrors.IsConflict(err) {
+		return false
+	}
+	statusErr, ok := err.(apierrors.APIStatus)
+	if !ok {
+		return false
+	}
+	status := statusErr.Status()
+	if status.Details == nil {
+		return false
+	}
+	if status.Details.Group != sessionResource.Group || status.Details.Kind != sessionResource.Resource {
+		return false
+	}
+	return strings.Contains(status.Message, "session target mismatch")
+}


@copilot apply changes based on this feedback

+	if sandbox.Name != "test-runtime" {
+		t.Errorf("expected Name test-runtime (the requested workload name), got %s", sandbox.Name)
+	}
+	if sandbox.SandboxNamespace != "default" {
+		t.Errorf("expected SandboxNamespace default, got %s", sandbox.SandboxNamespace)
+	}
+	if sandbox.Kind != types.AgentRuntimeKind {
+		t.Errorf("expected Kind %s, got %s", types.AgentRuntimeKind, sandbox.Kind)


The Docker build on CI uses --mount=type=cache for the Go build cache. Stale cache entries from a prior broken push were causing a phantom 'duplicate field name workloadName' error even though the source file was correct. Adding explanatory comments to the sandboxEntry literals in buildSandboxByAgentRuntime and buildSandboxByCodeInterpreter changes the file hash, forcing the Go toolchain to recompile the package from scratch and bypass the poisoned cache. The comments also document why WorkloadKind/WorkloadName are set here. Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

Copilot AI review requested due to automatic review settings May 11, 2026 18:55

volcano-sh-bot requested review from LiZhenCheng9527, YaoZengzeng and acsoto May 11, 2026 18:55

volcano-sh-bot added the size/L label May 11, 2026

Copilot started reviewing on behalf of KinshukSS2 May 11, 2026 18:56 View session

gemini-code-assist Bot reviewed May 11, 2026

View reviewed changes

volcano-sh-bot added the do-not-merge/invalid-commit-message label May 11, 2026

Copilot AI reviewed May 11, 2026

View reviewed changes

KinshukSS2 force-pushed the fix/session-route-validation branch from eff4b20 to a6ae2b8 Compare May 11, 2026 19:01

volcano-sh-bot removed the do-not-merge/invalid-commit-message label May 11, 2026

KinshukSS2 added 6 commits May 12, 2026 00:34

feat: add session target mismatch error handling and related tests

89b891d

Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

fix: correct formatting in TestGetSandboxBySession_Success

d91ac11

Signed-off-by: KinshukSS2 <kinshuk380@gmail.com>

KinshukSS2 force-pushed the fix/session-route-validation branch from a6ae2b8 to bd8b9a3 Compare May 11, 2026 19:05

Copilot AI review requested due to automatic review settings May 11, 2026 19:11

Copilot started reviewing on behalf of KinshukSS2 May 11, 2026 19:12 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

KinshukSS2 force-pushed the fix/session-route-validation branch from 2d5bf71 to b6d9492 Compare May 11, 2026 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(router): enforce session-to-route binding validation#324

fix(router): enforce session-to-route binding validation#324
KinshukSS2 wants to merge 7 commits into
volcano-sh:mainfrom
KinshukSS2:fix/session-route-validation

KinshukSS2 commented May 11, 2026

Uh oh!

volcano-sh-bot commented May 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot May 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

KinshukSS2 May 11, 2026

Uh oh!

KinshukSS2 May 11, 2026

Uh oh!

KinshukSS2 May 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

KinshukSS2 May 11, 2026

Uh oh!

KinshukSS2 May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

KinshukSS2 commented May 11, 2026

Problem

Solution

Changes

Testing

Uh oh!

volcano-sh-bot commented May 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

KinshukSS2 May 11, 2026

Choose a reason for hiding this comment

Uh oh!

KinshukSS2 May 11, 2026

Choose a reason for hiding this comment

Uh oh!

KinshukSS2 May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

KinshukSS2 May 11, 2026

Choose a reason for hiding this comment

Uh oh!

KinshukSS2 May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants