Skip to content

fix(server): handleExecute preserves caller requestContext.headers + explorer honors per-tool rules#68

Merged
encodedz merged 3 commits intomasterfrom
feat/odedgo/execute-merge-request-headers-and-explorer-tool-filter
May 5, 2026
Merged

fix(server): handleExecute preserves caller requestContext.headers + explorer honors per-tool rules#68
encodedz merged 3 commits intomasterfrom
feat/odedgo/execute-merge-request-headers-and-explorer-tool-filter

Conversation

@encodedz
Copy link
Copy Markdown
Collaborator

@encodedz encodedz commented May 5, 2026

Summary

Two isolated server-side fixes to ATP's handler surface. Both are bug fixes; neither introduces a new API or changes existing contracts for callers that were working.

1. handleExecute merges caller-supplied requestContext.headers

packages/server/src/handlers/execute.handler.ts

When a caller passes config.requestContext.headers through client.execute(code, config) (already a typed entry point via Record<string, unknown>), the in-process handler silently dropped them. Root cause: the executionConfig builder did:

requestContext: {
  ...requestConfig.requestContext,  // .headers present if caller set it
  headers: ctx.headers,             // overwrites — caller headers lost
  ...
}

Fix: merge the two header bags. Caller-supplied headers first, then ctx.headers takes precedence on overlapping keys. Non-conflicting caller entries survive; for conflicts, transport-layer session auth still wins so callers cannot spoof session headers.

 requestContext: {
   ...requestConfig.requestContext,
-  headers: ctx.headers,
+  headers: {
+    ...(requestConfig.requestContext as { headers?: Record<string, string> } | undefined)?.headers,
+    ...ctx.headers,
+  },
   path: ctx.path,
   method: ctx.method,
 },

Impact: purely additive. No caller that relied on the old behavior (silently dropped headers) can observe a regression, because silently-dropped data has no observable consequence. Callers that DID supply requestContext.headers and expected them to survive now get what they expected.

2. ExplorerService.getFilterContext iterates allowedGroups, not raw apiGroups

packages/server/src/explorer/index.ts

filterApiGroups(this.apiGroups) already returns per-tool-filtered groups — isToolAllowed correctly drops functions whose names are in blockTools / not in allowOnlyTools / carry a blocked operationType / blocked sensitivityLevel. The subsequent loop in getFilterContext then re-iterated the unfiltered this.apiGroups and re-added every function in any allowed group:

for (const group of this.apiGroups) {
  if (!context.allowedGroups.has(group.name)) continue;
  if (group.functions) {
    for (const func of group.functions) {
      context.allowedTools.add(`${group.name}:${func.name}`);  // unfiltered!
    }
  }
}

Net symptom: per-tool rules (allowOnlyTools, blockTools, blockOperationTypes, blockSensitivityLevels) worked correctly for the sandbox's api.* namespace AND for SearchEngine.search (which already iterates allowedGroups — the correct pattern). But explore_api returned every operation regardless. For downstream governance layers that rely on explore_api to serve a grant-filtered discovery surface, this was a silent no-op.

Fix: iterate the already-filtered allowedGroups. Mirrors the existing pattern in SearchEngine.search.

-for (const group of this.apiGroups) {
-  if (!context.allowedGroups.has(group.name)) continue;
+for (const group of allowedGroups) {
   if (group.functions) {
     for (const func of group.functions) {
       context.allowedTools.add(`${group.name}:${func.name}`);
     }
   }
 }

Live verification

Both fixes exercised via an end-to-end smoke in DaPulse/mcp-tools (ATP POC branch feat/odedgo/ocana-poc-mcp-gateway). With both applied, atp.explore_api correctly filters by per-tool grant rules, and atp.execute_code correctly forwards per-provider auth tokens from the caller through to the sandbox's outbound HTTPS calls.

Test plan

  • Unit test: calling handleExecute with config.requestContext.headers = { 'x-custom': 'abc' } results in an executionConfig.requestContext.headers containing 'x-custom': 'abc' (plus whatever was in ctx.headers).
  • Unit test: handleExecute with overlapping keys — config.requestContext.headers = { 'authorization': 'caller' } and ctx.headers = { 'authorization': 'session' } — final value is 'session' (ctx wins on conflict).
  • Unit test: ExplorerService.explore('/openapi/calendar/events') with runInRequestScope({ toolRules: { allowOnlyTools: ['calendar.events_list'] } })items contains exactly ['events_list'], not the full events resource list.
  • Regression check: existing SearchEngine.search per-tool filtering tests continue to pass (pattern borrowed from there).

🤖 Generated with Claude Code

encodedz and others added 3 commits May 5, 2026 13:45
When a caller passes `config.requestContext.headers` through
`client.execute(code, config)` (signature already supports this via
`Record<string, unknown>`), the in-process handler dropped them on the
floor. Root cause: the executionConfig builder did:

    requestContext: {
      ...requestConfig.requestContext,   // .headers present if caller set it
      headers: ctx.headers,              // overwrites with ctx headers
      ...
    }

The spread placed the caller's headers into `requestContext`, but the
next line replaced the entire `headers` field with `ctx.headers` —
silently losing the caller's payload.

Fix: merge the two header bags. Caller-supplied headers first, then
ctx.headers takes precedence for overlapping keys. Non-conflicting
caller entries (e.g. a governance layer's `X-ATP-<apiGroup>-token`)
now survive; for conflicts, transport-layer session auth still wins
so callers cannot spoof session auth.

Concrete motivating use case: a gateway-layer policy engine forwards
per-provider auth tokens into the sandbox via the documented
`requestContext.headers` entry point. Without this fix, the sandbox
reaches third-party APIs with no Authorization header despite the
caller having supplied one.

Behavioral impact: purely additive. No caller that relied on the old
behavior (silently dropped headers) can observe a regression, because
silently-dropped data has no observable consequence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`filterApiGroups(this.apiGroups)` already returns per-tool-filtered
groups — `isToolAllowed` correctly drops functions whose names are in
`blockTools` / not in `allowOnlyTools` / carry a blocked
`operationType` / blocked `sensitivityLevel`. The subsequent loop in
getFilterContext then re-iterated the UNFILTERED `this.apiGroups` and
re-added every function in any allowed group — silently defeating
every operation-level rule for the `explore_api` code path.

Net symptom: passing `toolRules: { allowOnlyTools: ['calendar.events_list'] }`
through the request body (body.toolRules → runInRequestScope) correctly
filtered `api.*` inside the sandbox AND correctly filtered
`search_api`'s output (search iterates the already-filtered groups),
but `explore_api` returned every operation regardless. Per-tool
governance was silently a no-op for explore.

Fix: iterate the already-filtered `allowedGroups` instead of the raw
`this.apiGroups`. One-line change matching the pattern `SearchEngine
.search` already uses. `this.apiGroups` remains the authoritative
enumeration used by `allGroups` / `allowedTypes`; `allowedTools` now
correctly sources from the filtered set.

Tests: existing `search` tests that assert per-tool filtering can be
duplicated for `explore`'s `items` output to lock this in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a server-side parity test asserting ExplorerService.explore and
SearchEngine.search surface the exact same visible operation set under
identical toolRules scenarios:

  - no rules → both show every function
  - allowOnlyApiGroups → both hide dropped groups identically
  - allowOnlyTools → both narrow to the exact per-op allowlist
  - blockTools → both drop the blocked ops identically
  - empty allowOnlyApiGroups → both unrestricted (documents the
    isGroupAllowed contract)

Lets callers move between search and explore without observing
different visible sets — the property the explorer fix was about.
5/5 passing.

Also shortens the inline comments added to getFilterContext and
handleExecute in the earlier two commits — both now 2-3 lines
instead of a paragraph.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
// so both should see every function. Documenting that parity explicitly.
const s = await parityUnder(rules, () => searchVisible(search));
const e = parityUnder(rules, () => exploreVisible(explorer));
expect(e).toEqual(s);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you forgot to add an assertion that all tools exist

@encodedz encodedz merged commit 17b678d into master May 5, 2026
3 checks passed
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.

2 participants