Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ graph LR
| Web UI + macOS app UX audit | In progress | P0 | — | | |
| Release qualification gate (auto-QA matrix blocks the tag) | In progress | P0 | — | [081-release-qa-gate](./specs/081-release-qa-gate/) | |
| Action log / transparency — info at a glance | In progress | P1 | — | | |
| Agent-token scope hardening: every MCP request authorized by its own scope (spec 105) | In progress | P1 | 68/111 (61%) | [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | |
| Agent-token scope hardening: every MCP request authorized by its own scope (spec 105) | In progress | P1 | 82/112 (73%) | [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | |
| Token-efficiency benchmark: measured savings, published results | In progress | P1 | 62/64 (97%) | [103-token-bench](./specs/103-token-bench/) | |
| Telemetry identity & data quality (machine_id + CI-filter hardening) | In progress | P1 | — | | |
| Telemetry v7: honest funnel + churn instrumentation | In progress | P1 | — | [080-telemetry-v7-churn](./specs/080-telemetry-v7-churn/) | |
Expand Down Expand Up @@ -1034,6 +1034,6 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—`
| [102-schema-deferred](./specs/102-schema-deferred/) | `shipped` | 89/89 (100%) |
| [103-token-bench](./specs/103-token-bench/) | `shipped` | 62/64 (97%) |
| [104-auto-routing-mode](./specs/104-auto-routing-mode/) | — | — |
| [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | `in-flight` | 68/111 (61%) |
| [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | `in-flight` | 82/112 (73%) |
| [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) |
| [107-server-edition-sso-hardening](./specs/107-server-edition-sso-hardening/) | `shipped` | 126/126 (100%) |
51 changes: 42 additions & 9 deletions internal/jsruntime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ type AuthInfo struct {
Permissions []string // Permission tiers: "read", "write", "destructive"
}

// isAdmin reports whether this identity is one of the two administrator
// AuthInfo.Type values — the same short-circuit CanAccessServer and
// HasPermission apply internally, promoted to its own predicate (Spec 105
// FR-010 gap G7) so a caller outside those two methods can ask the same
// question without re-deriving it. A nil receiver is NOT an administrator
// here (unlike the two methods above, whose nil-tolerant "everything is
// allowed" default exists for the STDIO/in-process caller that carries no
// AuthInfo at all — a different case from "this AuthInfo IS one").
func (a *AuthInfo) isAdmin() bool {
return a != nil && (a.Type == "admin" || a.Type == "admin_user")
}

// CanAccessServer checks whether this auth context can access the named server.
func (a *AuthInfo) CanAccessServer(name string) bool {
if a == nil || a.Type == "admin" || a.Type == "admin_user" {
Expand Down Expand Up @@ -542,19 +554,40 @@ func (ec *ExecutionContext) resolveDispatchGates(serverName, toolName string, ar
// Check allowed servers. When restrictToAllowed is set (active Spec 057
// profile), the map is enforced even when empty — an empty effective set
// means "deny everything". Otherwise an empty map means "no restriction".
if (ec.restrictToAllowed || len(ec.allowedServerMap) > 0) && !ec.allowedServerMap[serverName] {
//
// Spec 105 FR-010 gap G7: when this execution carries a real agent
// token, the profile-derived allowedServerMap and the token's OWN server
// scope are two independent restrictions on the SAME effective set
// (profile ∩ token) and must answer with ONE refusal regardless of
// which one excludes a given server — a server inside the profile pin
// but outside the token, and a server outside BOTH (or nonexistent),
// must be indistinguishable. Evaluate both before returning either error
// so the ONE body always used for an agent caller (ErrorCodeAccessDenied)
// is what a bare profile-map miss also gets.
//
// "carries a real agent token" is NOT "authInfo != nil": mcp_code_
// execution.go's applyProfileScopeToExecution populates AuthInfo for
// EVERY authenticated caller, administrators included (an HTTP admin's
// AuthInfo.Type is "admin"/"admin_user"). Routing an admin through the
// agent-only branch above would rename a profile-only exclusion's
// wording to the token-scope body even though the admin holds no token
// to conflate it with — a caller-kind regression codex round-1 review
// caught. isAdmin() is the same short-circuit CanAccessServer and
// HasPermission already apply internally. Administrators — real ones
// (authInfo.isAdmin()) and the stdio/in-process caller that carries no
// AuthInfo at all (authInfo == nil) — both fall through to the
// profile-only branch below, unchanged from pre-105.
profileDenies := (ec.restrictToAllowed || len(ec.allowedServerMap) > 0) && !ec.allowedServerMap[serverName]
if ec.authInfo != nil && !ec.authInfo.isAdmin() {
if profileDenies || !ec.authInfo.CanAccessServer(serverName) {
ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, "", args)
return errorEnvelope(ErrorCodeAccessDenied, fmt.Sprintf("token does not have access to server '%s'", serverName)), "", nil
}
} else if profileDenies {
ec.reportAuthzRefusal(serverName, toolName, ErrorCodeServerNotAllowed, "", args)
return errorEnvelope(ErrorCodeServerNotAllowed, fmt.Sprintf("server not allowed: %s", serverName)), "", nil
}

// Auth context enforcement (Spec 031): the token's server scope answers
// before anything about the tool is looked up, so an out-of-scope server
// is refused without disclosing whether the name resolves on it.
if ec.authInfo != nil && !ec.authInfo.CanAccessServer(serverName) {
ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, "", args)
return errorEnvelope(ErrorCodeAccessDenied, fmt.Sprintf("token does not have access to server '%s'", serverName)), "", nil
}

// Determine required permission via annotation lookup. The gate-capturing
// form is the ONE read of the nested call: its tier decides the checks
// below and its gate is what the dispatch runs on.
Expand Down
83 changes: 83 additions & 0 deletions internal/jsruntime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,89 @@ func TestExecuteAuthContext_ServerAccessDenied(t *testing.T) {
}
}

// Spec 105 PR G, FR-010 gap G7 — codex round-1 review MUST-FIX: an
// administrator (or admin_user) carrying a profile restriction must keep the
// PRE-fix ErrorCodeServerNotAllowed / "server not allowed: ..." wording, not
// the agent-token ErrorCodeAccessDenied / "token does not have access ..."
// body the unified profile∩token check introduced. AuthInfo is populated for
// EVERY authenticated HTTP caller (mcp_code_execution.go's
// applyProfileScopeToExecution), administrators included, so `authInfo !=
// nil` alone is not "this is an agent token" — only Type=="agent" (or
// "user") is a real token with its own AllowedServers to intersect against
// the profile map; Type=="admin"/"admin_user" has no such token, and
// resolveDispatchGates must treat it exactly like the no-AuthInfo
// stdio/in-process case.
func TestExecuteAuthContext_ProfileScopedAdmin_KeepsServerNotAllowedWording(t *testing.T) {
for _, adminType := range []string{"admin", "admin_user"} {
t.Run(adminType, func(t *testing.T) {
caller := newMockToolCaller()
code := `
var res = call_tool("weather", "get_forecast", {});
({ ok: res.ok, code: res.error ? res.error.code : null, msg: res.error ? res.error.message : null })
`
opts := ExecutionOptions{
// The profile restricts the sandbox to "github" only —
// "weather" is outside the profile, not the (nonexistent)
// token scope.
RestrictToAllowed: true,
AllowedServers: []string{"github"},
AuthContext: &AuthInfo{
Type: adminType,
// An admin's AuthInfo carries no AllowedServers/
// Permissions restriction of its own (CanAccessServer/
// HasPermission both short-circuit true for this Type) —
// exactly what makes this cell distinguishable from an
// agent token's.
},
}

result := Execute(context.Background(), caller, code, opts)
if !result.Ok {
t.Fatalf("expected ok=true (the script itself must run), got error: %v", result.Error)
}
resultMap := result.Value.(map[string]interface{})
if resultMap["ok"] != false {
t.Fatalf("expected the nested call to be refused, got ok=%v", resultMap["ok"])
}
if resultMap["code"] != string(ErrorCodeServerNotAllowed) {
t.Errorf("expected %s (unchanged pre-105 wording for a %s), got %v (%v)",
ErrorCodeServerNotAllowed, adminType, resultMap["code"], resultMap["msg"])
}
// codex round-2 review, MUST-FIX: the previous `if msg != ""`
// guard made this assertion vacuously pass for an empty,
// missing, or non-string "msg" — require the exact wording
// unconditionally.
msg, ok := resultMap["msg"].(string)
if !ok {
t.Fatalf("expected msg to be a string, got %T (%v)", resultMap["msg"], resultMap["msg"])
}
if msg != "server not allowed: weather" {
t.Errorf("expected the profile-only wording, got %q", msg)
}
if len(caller.calls) != 0 {
t.Errorf("expected 0 upstream calls, got %d", len(caller.calls))
}

// Control: a server INSIDE the profile still dispatches for this
// admin — the profile restriction itself is still enforced, only
// its wording (and refusal code) must not shift to the
// agent-token body.
controlCode := `
var res = call_tool("github", "list_repos", {});
({ ok: res.ok })
`
controlResult := Execute(context.Background(), caller, controlCode, opts)
if !controlResult.Ok {
t.Fatalf("control: expected ok=true, got error: %v", controlResult.Error)
}
controlMap := controlResult.Value.(map[string]interface{})
if controlMap["ok"] != true {
t.Fatalf("control: an in-profile server must still dispatch for this %s, got %v", adminType, controlMap)
}
})
}
}

// TestExecuteAuthContext_PermissionDenied tests auth enforcement blocks insufficient permissions
func TestExecuteAuthContext_PermissionDenied(t *testing.T) {
caller := newMockToolCaller()
Expand Down
56 changes: 42 additions & 14 deletions internal/server/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2573,7 +2573,25 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp.
// never re-derived here — cacheAuthorizationWith's caller-intersected
// ProfileServers stamp needs it (Spec 105 PR D review round 17).
producer := p.cacheAuthorizationWith(ctx, profileSlug, profileScope, profileIdx)
if profileScope != nil && !profileScope.Allows(serverName) {
// Spec 105 FR-010 gap G7: effective scope is profile ∩ token, checked in
// ONE evaluation with ONE refusal body for a scoped agent caller — never
// the profile-only check alone, which used to run here regardless of
// caller kind and could answer a DIFFERENT body ("server 'b' is not in
// profile 'P'") than the token-scope check further below ("Server 'b' is
// not in scope for this agent token") for the SAME server, depending on
// which of the two excluded it. A server inside the pin but outside the
// token (or the reverse) must be indistinguishable from a nonexistent
// one. Administrators keep today's profile-only text unchanged — they
// have no token scope to intersect with.
scopeAuthCtx := auth.AuthContextFromContext(ctx)
scopedCallerForScope := scopeAuthCtx != nil && !scopeAuthCtx.IsAdmin()
if scopedCallerForScope {
if !p.serverInScope(scopeAuthCtx, profileScope, serverName) {
errMsg := fmt.Sprintf("Server '%s' is not in scope for this agent token", serverName)
p.emitActivityPolicyDecision(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonTokenScope)
return mcp.NewToolResultError(errMsg), nil
}
} else if profileScope != nil && !profileScope.Allows(serverName) {
errMsg := fmt.Sprintf("server '%s' is not in profile '%s'", serverName, profileScope.Name)
p.emitActivityPolicyDecision(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonProfileScope)
return mcp.NewToolResultError(errMsg), nil
Expand Down Expand Up @@ -2621,19 +2639,14 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp.
p.dispatchGatePause(serverName, actualToolName)
}

// Spec 028: Enforce agent token scope restrictions. The server-scope and
// variant-permission gates run before the identity gate so a scoped
// caller learns nothing about a server outside its scope from the shape
// of the refusal.
authCtx := auth.AuthContextFromContext(ctx)
scopedCaller := authCtx != nil && !authCtx.IsAdmin()
// Spec 028: Enforce agent token scope restrictions. The server-scope gate
// above (effective scope = profile ∩ token, Spec 105 FR-010 G7) already
// ran before the identity gate so a scoped caller learns nothing about a
// server outside its scope from the shape of the refusal; only the
// variant-permission gate remains here.
authCtx := scopeAuthCtx
scopedCaller := scopedCallerForScope
if scopedCaller {
// Check server scope
if !authCtx.CanAccessServer(serverName) {
errMsg := fmt.Sprintf("Server '%s' is not in scope for this agent token", serverName)
p.emitActivityPolicyDecision(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonTokenScope)
return mcp.NewToolResultError(errMsg), nil
}
// Check permission scope — map tool variant to required permission
var requiredPerm string
switch toolVariant {
Expand Down Expand Up @@ -2886,8 +2899,23 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp.
// the admission queue and immediately before the transport.
certified = live
} else {
// Get list of available servers for helpful error message
// Get list of available servers for helpful error message. Spec 105
// FR-010 G1/T100: for a scoped agent caller this list is filtered
// through the SAME effective-scope predicate (serverInScope =
// profile ∩ token) the dispatch gates above already evaluated — an
// unauthorized server's mere existence must not leak into a hint
// meant to help the caller find ITS OWN servers. Administrators are
// unaffected: their branch below is untouched.
availableServers := p.upstreamManager.GetAllServerNames()
if scopedCaller {
visible := make([]string, 0, len(availableServers))
for _, name := range availableServers {
if p.serverInScope(authCtx, profileScope, name) {
visible = append(visible, name)
}
}
availableServers = visible
}
serverList := strings.Join(availableServers, ", ")
if len(availableServers) == 0 {
serverList = "(no servers configured)"
Expand Down
Loading
Loading