Repository navigation
fix(harness): route the tenant key through credentialFetch [SAP-4112] - #1257
antoine-berger wants to merge 6 commits into
Conversation
…[SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ers [SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…the allowlist [SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Credential contract and policy packages/analytics-core/src/credential.ts, packages/analytics-core/src/credential.spec.ts, packages/harness/src/core/credential-policy.ts, scripts/credential-header-allowlist.json, .changeset/harness-credential-fetch.md, plans/credential-transport/..., packages/harness/README.md |
Analytics Core adds the "authorization" credential header and credentialHeaders(), which checks the URL before returning a credential header. Harness adds a policy resolved per call. Tests cover authorization-header replacement and redirect behavior, plus URL policy cases. The changeset and README document the transport policy. |
Harness API request migration packages/harness/src/core/account-plan*, packages/harness/src/core/definition-app-link*, packages/harness/src/core/definition-slug-resolver*, packages/harness/src/core/run-state*, packages/harness/src/core/template-catalog*, packages/harness/src/core/vault-secrets*, packages/harness/src/server/runs.test.ts |
Harness readers and fetchers use credentialFetch with the harness policy. Tests cover refusal of non-loopback plaintext HTTP, opt-in behavior, and existing request handling. |
MCP configuration and Assistant bridge packages/harness/src/core/inject/mcp-config*, packages/harness/src/server/opencode-bridge.ts |
MCP configuration uses credentialHeaders() for a present API key. Tests cover remote HTTP refusal, keyless configuration, loopback HTTP, and opted-in HTTP. The bridge passes the API key to credentialFetch instead of adding it to ordinary upstream request headers. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant HarnessRequest
participant harnessCredentialPolicy
participant credentialFetch
participant fetchImpl
HarnessRequest->>harnessCredentialPolicy: Resolve policy for request
HarnessRequest->>credentialFetch: Pass URL, credential, and policy
credentialFetch->>fetchImpl: Send request when URL policy permits
Merge Risk: 
Minimal · up to b8d96
-
No actionable issue is established for the documentation change. The PR is mergeable subject to normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Description check
-
Explanation
The description follows the required template and covers the problem, scope, related work, validation commands, tests, documentation, compatibility impact, release changes, security declarations, AI assistance, and checklist items.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 19 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the path before the key can roam,
HTTPS or loopback leads it safely home.
Across new origins, the secret stays behind,
While opted-in HTTP follows policy’s line.
The harness and core now share the same design.
Comment @coderabbitai help to get the list of available commands.
…ter test [SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… and desktop [SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n sessions [SAP-4112] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Primary change type
Problem and motivation
Since SAP-4106 every SDK client sends the tenant API key through
credentialFetch, which refuses plain http to a non-loopback host and drops the key on a redirect to another origin. Agent Studio (@sapiom/harness) was left out: eight requests and the MCP config it writes for Claude Code and Codex still set the key header by hand, andscripts/credential-header-allowlist.jsonexempted four of them from the CI guard. A custom plain-http environment, or a redirect, could send the key in clear from Studio while the rest of the SDK refuses.Summary and scope
@sapiom/analytics-core/credential:Credential.headeraccepts"authorization"(Bearer APIs; the caller's ownAuthorizationis replaced and the header is dropped on a cross-origin hop, as for the other two). NewcredentialHeaders(url, credential, policy)checks a URL before a key is handed to another process.@sapiom/harness: newharnessCredentialPolicy()(label@sapiom/harness, opt-inSAPIOM_ALLOW_INSECURE_HTTP=1, read per call).x-sapiom-api-key):run-state.ts,definition-slug-resolver.ts(2 sites).authorization: Bearer):vault-secrets.ts,account-plan.ts,template-catalog.ts,definition-app-link.ts.opencode-bridge.ts): samex-api-key, stillredirect: "error".mcp-config.ts: the remote MCP key is only written for an allowed URL; otherwise session start fails with the refusal message, which names the opt-in.NO_APP_LINK,VaultSecretError).scripts/credential-header-allowlist.jsonis now[].Out of scope:
studio-credentials.tsandassistant-access.tssend the Studio OAuth token, not the tenant key. The harness analytics emitter already goes throughcredentialFetch. A Studio-specific opt-in (CLI flag or desktop setting) is not added: the env var is the only switch.Plan:
plans/credential-transport/harness-tenant-key-credential-fetch.md.Related work
Related issue or discussion: SAP-4112 (follows SAP-4106, #1236)
Validation
Tests and documentation
credential.spec.ts:authorizationcredential replaces a caller header, is dropped cross-origin and kept same-origin;credentialHeadersfor https, loopback, LAN http, a non-http scheme and the opt-in.SAPIOM_ALLOW_INSECURE_HTTP=1.authorizationheader andobjectContaining(credentialFetch addsredirect).Compatibility and release impact
~/.sapiom/credentials.jsonenvironment on plain http to a non-loopback host (Docker service name, LAN IP) now get a refused session start and unavailable Agents/core panels until they setSAPIOM_ALLOW_INSECURE_HTTP=1. Built-in environments are https andlocalhostis unchanged.@sapiom/harnessminor,@sapiom/analytics-coreminor).Security
will follow the
Security Policy for
private reporting.
AI assistance
Claude Code (Opus) wrote the plan, the code and the tests from SAP-4112; I reviewed the diff and the commands above were run locally.
Checklist
CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.any N/A checks above.
🤖 Generated with Claude Code
Summary by CodeRabbit
SAPIOM_ALLOW_INSECURE_HTTP=1to allow it on trusted networks.