From d562a569c047face5f2b5f5731dba96c7cbd7826 Mon Sep 17 00:00:00 2001 From: "CTO (Paperclip agent)" Date: Sun, 2 Aug 2026 09:35:53 +0000 Subject: [PATCH 1/5] fix(gh-wrapper): rename the seat-token env key out of the PAPERCLIP_ namespace (BLO-18927) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #830/#841 added a volume-free delivery path to the gh wrapper so a GitHub credential can be bound per-agent instead of mounted into every agent pod. It has never been reachable. `isPaperclipRuntimeEnvKey` (server/src/services/heartbeat.ts) strips every `PAPERCLIP_*` key out of adapter, environment, project and routine env, and agent-scope binding resolution reads that already-stripped config. So a binding at `env.PAPERCLIP_GITHUB_TOKEN_VALUE` is deleted server-side at every scope before it can reach a pod, and the wrapper falls through to the fleet-wide mounted file as if nothing were configured — silently, with no error on either side. Rename the wrapper's variable to `GH_SEAT_TOKEN_VALUE`. The guard itself is correct and stays untouched: it exists to stop user config overriding paperclip's own runtime env, and punching a credential-shaped exception into it would be the wrong direction. The credential moves out of the namespace instead. Adds a regression test asserting the key survives agent-scope resolution, paired with a `PAPERCLIP_`-prefixed control in the same env block so a future change neutering the strip cannot make it pass for the wrong reason. No behavioural change to any working configuration: the old name could never have been populated, so nothing can be depending on it. Refs BLO-18927 --- scripts/gh-token-wrapper.sh | 41 +++++++++++---- scripts/gh-token-wrapper.test.mjs | 18 +++---- .../__tests__/heartbeat-project-env.test.ts | 52 +++++++++++++++++++ 3 files changed, 93 insertions(+), 18 deletions(-) diff --git a/scripts/gh-token-wrapper.sh b/scripts/gh-token-wrapper.sh index 3b68d39ca61e..32c686e55129 100755 --- a/scripts/gh-token-wrapper.sh +++ b/scripts/gh-token-wrapper.sh @@ -14,13 +14,13 @@ # existing. # # A second, volume-free delivery path exists for credentials bound per-agent -# rather than mounted fleet-wide: see PAPERCLIP_GITHUB_TOKEN_VALUE below. +# rather than mounted fleet-wide: see GH_SEAT_TOKEN_VALUE below. set -eu TOKEN_FILE="${PAPERCLIP_GITHUB_TOKEN_FILE:-/paperclip/.secrets/github-token/token}" REAL_GH="${GH_TOKEN_WRAPPER_REAL_GH:-/usr/bin/gh.real}" -# PAPERCLIP_GITHUB_TOKEN_VALUE carries a token *value* rather than a path, for +# GH_SEAT_TOKEN_VALUE carries a token *value* rather than a path, for # credentials delivered by the scoped secret-binding path (per-agent / # per-project env bindings) instead of by a mounted secret volume. It exists so # a credential can be given to specific agents without mounting it into every @@ -28,19 +28,42 @@ REAL_GH="${GH_TOKEN_WRAPPER_REAL_GH:-/usr/bin/gh.real}" # every Job pod with no agent or tenant filter (BLO-18927, BLO-18970), so a # volume-delivered secret is necessarily fleet-wide. # +# The name deliberately does NOT start with `PAPERCLIP_`. Do not "fix" it for +# consistency with the FILE variable below — the prefix is load-bearing in the +# opposite direction. `isPaperclipRuntimeEnvKey` (server/src/services/ +# heartbeat.ts) strips every `PAPERCLIP_*` key out of adapter, environment, +# project and routine env, and agent-scope binding resolution reads that +# already-stripped config. A `PAPERCLIP_`-prefixed name is therefore +# unreachable from the very binding path this branch exists to serve: it would +# be silently deleted server-side and fall through to the file branch with no +# error anywhere. That guard is correct and must stay — it stops user config +# overriding paperclip's own runtime env — so the credential moves out of its +# namespace instead. See BLO-18927 step 2. +# +# The FILE variable keeps its `PAPERCLIP_` prefix on purpose, for the same +# reason inverted: it selects a path on disk, and being strippable is what +# stops project/environment config from redirecting the file branch. +# # Precedence: value > file. Both are *explicit caller selections* of an identity # for one invocation — the same trust model the FILE variable already had, since -# a caller could always point that at a file it wrote. This is deliberately NOT -# GH_TOKEN: the override below must keep clobbering GH_TOKEN unconditionally -# (BLO-13241), so GH_TOKEN cannot double as an input without reopening that bug. -if [ "${PAPERCLIP_GITHUB_TOKEN_VALUE+x}" = x ]; then +# a caller could always point that at a file it wrote. Note the widened reach +# that follows from the rename: a non-`PAPERCLIP_` key can now be set from +# project/environment/routine env, not just agent scope. That is a downgrade +# vector, not an escalation one — it can only swap in a credential the setter +# already holds, never read the mounted one — but it does mean `gh` identity +# selection is only as tight as write access to those env scopes. +# +# This is deliberately NOT GH_TOKEN: the override below must keep clobbering +# GH_TOKEN unconditionally (BLO-13241), so GH_TOKEN cannot double as an input +# without reopening that bug. +if [ "${GH_SEAT_TOKEN_VALUE+x}" = x ]; then # Trim surrounding whitespace only — a secret that arrives via a templated # env binding routinely picks up a trailing newline, and tabs/spaces are just # as likely as CR/LF from a YAML block scalar. Deliberately a trim rather than # a delete: `tr -d` would silently splice "ghu_aaa\nbbb" into the single # plausible-looking token "ghu_aaabbb" and authenticate as nobody-in- # particular, which is the failure mode this whole branch exists to avoid. - TOKEN="${PAPERCLIP_GITHUB_TOKEN_VALUE}" + TOKEN="${GH_SEAT_TOKEN_VALUE}" while :; do case "${TOKEN}" in [[:space:]]*) TOKEN="${TOKEN#?}" ;; @@ -54,7 +77,7 @@ if [ "${PAPERCLIP_GITHUB_TOKEN_VALUE+x}" = x ]; then # continuing would run `gh` under whatever ambient GH_TOKEN/GITHUB_TOKEN the # caller happened to inherit — an unintended identity, silently. if [ -z "${TOKEN}" ]; then - echo "gh-token-wrapper: PAPERCLIP_GITHUB_TOKEN_VALUE is set but holds only whitespace; refusing to run with ambient auth" >&2 + echo "gh-token-wrapper: GH_SEAT_TOKEN_VALUE is set but holds only whitespace; refusing to run with ambient auth" >&2 exit 64 fi case "${TOKEN}" in @@ -62,7 +85,7 @@ if [ "${PAPERCLIP_GITHUB_TOKEN_VALUE+x}" = x ]; then # No GitHub token format contains whitespace, so this is either a # concatenation of two values or a corrupted binding. Refuse rather than # guess which half was meant. The value itself is never echoed. - echo "gh-token-wrapper: PAPERCLIP_GITHUB_TOKEN_VALUE contains embedded whitespace; refusing to guess at the intended token" >&2 + echo "gh-token-wrapper: GH_SEAT_TOKEN_VALUE contains embedded whitespace; refusing to guess at the intended token" >&2 exit 64 ;; esac diff --git a/scripts/gh-token-wrapper.test.mjs b/scripts/gh-token-wrapper.test.mjs index b096553553ce..a449acedc9a5 100644 --- a/scripts/gh-token-wrapper.test.mjs +++ b/scripts/gh-token-wrapper.test.mjs @@ -29,13 +29,13 @@ function withTempDir(fn) { // branch has to clear all of them, because the wrapper's whole job is to pick a // branch based on which are set — inheriting one from the ambient environment // silently re-points the test at a different branch than it names. This is not -// hypothetical: these tests run inside agent pods, and PAPERCLIP_GITHUB_TOKEN_VALUE +// hypothetical: these tests run inside agent pods, and GH_SEAT_TOKEN_VALUE // is exactly what the scoped secret-binding path (BLO-18927) exports there. const WRAPPER_CREDENTIAL_ENV_VARS = [ "GH_TOKEN", "GITHUB_TOKEN", "PAPERCLIP_GITHUB_TOKEN_FILE", - "PAPERCLIP_GITHUB_TOKEN_VALUE", + "GH_SEAT_TOKEN_VALUE", ]; // The single way any test in this file builds an environment. Starts from a @@ -57,7 +57,7 @@ function runWrapper(dir, { tokenFileContent, tokenValue, args = ["api", "user"] const env = sanitizedEnv({ GH_TOKEN_WRAPPER_REAL_GH: stubGhPath }); if (tokenValue !== undefined) { - env.PAPERCLIP_GITHUB_TOKEN_VALUE = tokenValue; + env.GH_SEAT_TOKEN_VALUE = tokenValue; } if (tokenFileContent !== undefined) { @@ -183,7 +183,7 @@ test("logs a diagnostic to stderr and falls back when the token file exists but }); }); -// PAPERCLIP_GITHUB_TOKEN_VALUE — credentials delivered by the scoped +// GH_SEAT_TOKEN_VALUE — credentials delivered by the scoped // secret-binding path rather than a mounted secret volume (BLO-18927). test("exports a token supplied by value when no token file exists", () => { @@ -232,7 +232,7 @@ function runMalformedValue(dir, tokenValue) { env: sanitizedEnv({ GH_TOKEN_WRAPPER_REAL_GH: stubGhPath, PAPERCLIP_GITHUB_TOKEN_FILE: tokenFilePath, - PAPERCLIP_GITHUB_TOKEN_VALUE: tokenValue, + GH_SEAT_TOKEN_VALUE: tokenValue, GH_TOKEN: "user_supplied_override", GITHUB_TOKEN: "user_supplied_override", }), @@ -252,7 +252,7 @@ for (const [label, tokenValue] of [ withTempDir((dir) => { const proc = runMalformedValue(dir, tokenValue); assert.equal(proc.status, 64); - assert.match(proc.stderr, /PAPERCLIP_GITHUB_TOKEN_VALUE is set but holds only whitespace/); + assert.match(proc.stderr, /GH_SEAT_TOKEN_VALUE is set but holds only whitespace/); assert.equal(proc.stdout, ""); }); }); @@ -294,7 +294,7 @@ test("a token supplied by value overrides a pre-existing GH_TOKEN in the caller' const env = sanitizedEnv({ GH_TOKEN_WRAPPER_REAL_GH: stubGhPath, - PAPERCLIP_GITHUB_TOKEN_VALUE: "ghu_userseat", + GH_SEAT_TOKEN_VALUE: "ghu_userseat", GH_TOKEN: "user_supplied_override", GITHUB_TOKEN: "user_supplied_override", }); @@ -353,7 +353,7 @@ test("Dockerfile.runtime points git's credential helper at the wrapper, not gh.r // environment. Before the sanitized-env helper, that run failed two tests: the // GH_TOKEN-override test authenticated as the inherited value instead of the // file's, and the unreadable-file test never emitted its diagnostic, because -// the inherited PAPERCLIP_GITHUB_TOKEN_VALUE sent both down the value branch. +// the inherited GH_SEAT_TOKEN_VALUE sent both down the value branch. // A plain assertion inside a single test cannot catch that class of bug — the // leak is in how each test builds its environment, so the check has to be a // second run of every test under a dirty one. @@ -366,7 +366,7 @@ if (!process.env.GH_TOKEN_WRAPPER_TEST_NESTED) { GH_TOKEN: "ambient_caller_token", GITHUB_TOKEN: "ambient_caller_token", PAPERCLIP_GITHUB_TOKEN_FILE: path.join(os.tmpdir(), "ambient-token-does-not-exist"), - PAPERCLIP_GITHUB_TOKEN_VALUE: "ghu_ambient_scoped_binding", + GH_SEAT_TOKEN_VALUE: "ghu_ambient_scoped_binding", }; // node:test sets NODE_TEST_CONTEXT=child-v8 in every test-file subprocess. // Inheriting it makes the nested run report through the v8 serializer to a diff --git a/server/src/__tests__/heartbeat-project-env.test.ts b/server/src/__tests__/heartbeat-project-env.test.ts index 8de22041400b..221a8a4506d2 100644 --- a/server/src/__tests__/heartbeat-project-env.test.ts +++ b/server/src/__tests__/heartbeat-project-env.test.ts @@ -208,6 +208,58 @@ describe("resolveExecutionRunAdapterConfig", () => { expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_"); }); + // Companion to the test above, and the reason GH_SEAT_TOKEN_VALUE is spelled + // without a `PAPERCLIP_` prefix (BLO-18927 step 2). The strip above is + // correct and stays; what it means is that any credential delivered by the + // scoped-binding path must live outside that namespace, because the strip + // runs *before* agent-scope resolution and removes the key with no error + // anywhere. A `PAPERCLIP_`-prefixed seat token is therefore not "a binding + // that sometimes fails" — it can never arrive at all, and the gh wrapper + // falls through to the fleet-wide mounted file as if nothing were configured. + // + // This asserts the key reaches resolveAdapterConfigForRuntime, which is the + // precise thing renaming it back would break. It is deliberately paired with + // a PAPERCLIP_-prefixed control in the same env block so that a future change + // making the strip a no-op cannot make this test pass for the wrong reason. + it("preserves the non-PAPERCLIP_ scoped seat-token key through agent-scope resolution", async () => { + const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record) => ({ + config: { + ...config, + env: { ...(config.env as Record) }, + }, + secretKeys: new Set(), + manifest: [], + })); + const resolveEnvBindings = vi.fn(async (_companyId, env: Record) => ({ + env: Object.fromEntries( + Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"), + ), + secretKeys: new Set(), + manifest: [], + })); + + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { + env: { + GH_SEAT_TOKEN_VALUE: { type: "secret_ref", secretId: "secret-seat-token", version: "latest" }, + // Control: same block, same scope, only the prefix differs. + PAPERCLIP_GITHUB_TOKEN_VALUE: { type: "secret_ref", secretId: "secret-seat-token", version: "latest" }, + }, + }, + secretsSvc: { + resolveAdapterConfigForRuntime, + resolveEnvBindings, + } as any, + }); + + const agentEnvSeenByResolver = (resolveAdapterConfigForRuntime.mock.calls[0]?.[1] as any)?.env ?? {}; + expect(agentEnvSeenByResolver).toHaveProperty("GH_SEAT_TOKEN_VALUE"); + expect(agentEnvSeenByResolver).not.toHaveProperty("PAPERCLIP_GITHUB_TOKEN_VALUE"); + expect(result.resolvedConfig.env).toHaveProperty("GH_SEAT_TOKEN_VALUE"); + }); + it("skips project env resolution when the project has no bindings", async () => { const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ config: { env: { AGENT_ONLY: "agent-only" } }, From 75be1c6f90384e9869f021d6395bc7408ac2db67 Mon Sep 17 00:00:00 2001 From: "CTO (Paperclip agent)" Date: Sun, 2 Aug 2026 11:31:22 +0000 Subject: [PATCH 2/5] fix(heartbeat): make GH_SEAT_TOKEN_VALUE agent-scope-only and a push credential Addresses both Important findings from Ally's review of #955. 1. [tests/errors] The seat key survived resolution but could not satisfy the push-capability preflight, which accepted only GH_TOKEN/GITHUB_TOKEN. An agent bound exactly as BLO-18927 step 3 intends was rejected as push_write_credential_missing before the wrapper could convert it. Add GH_SEAT_TOKEN_VALUE to PUSH_CAPABILITY_ENV_KEYS and export the constant so the test asserts the production contract instead of restating it. Scope note: requiresPushCapabilityPreflight gates on GIT_SENSITIVE_LOCAL_ADAPTER_TYPES, so this bit only local adapters, not the k8s adapters most PR authors run. The finding is real; its blast radius was narrower than stated. 2. [gstack/security] The rename out of the PAPERCLIP_ namespace also made the key settable from environment/project/routine env, which are overlaid AFTER agent-scope resolution -- so the lowest-trust writer won. Because the wrapper prefers this value over the mounted App token, such a writer could swap the identity every `gh` call runs as, or park whitespace there and fail them all with exit 64. Add AGENT_SCOPE_ONLY_ENV_KEYS, stripped from those three scopes and only those, restoring the protection the prefix used to give for free. Deliberately not folded into isPaperclipRuntimeEnvKey: that guard strips at every scope including agent, which is exactly what this key must escape. Also found while fixing 2: GH_SEAT_TOKEN_VALUE matches none of the name-shaped substrings in LOW_TRUST_SENSITIVE_ENV_KEY_RE, so a low-trust run could have inlined the raw seat credential. Treat agent-scope-only keys as sensitive explicitly. Zero regression risk -- the key is introduced by this PR, so no existing config can depend on the inline form. Tests: 3 new overlay/scope tests, 4 preflight tests, 1 low-trust test. Each new guard mutation-checked in isolation -- reverting the overlay strip fails exactly the 3 scope tests, reverting the contract fails exactly the preflight test, reverting the low-trust rule fails exactly that test. heartbeat-project-env 24/24, gh-token-wrapper 23/23, tsc --noEmit clean. Refs BLO-18927. --- scripts/gh-token-wrapper.sh | 17 +- .../__tests__/heartbeat-project-env.test.ts | 195 ++++++++++++++++++ server/src/services/heartbeat.ts | 64 +++++- 3 files changed, 264 insertions(+), 12 deletions(-) diff --git a/scripts/gh-token-wrapper.sh b/scripts/gh-token-wrapper.sh index 32c686e55129..8f4aeaab5ace 100755 --- a/scripts/gh-token-wrapper.sh +++ b/scripts/gh-token-wrapper.sh @@ -46,12 +46,17 @@ REAL_GH="${GH_TOKEN_WRAPPER_REAL_GH:-/usr/bin/gh.real}" # # Precedence: value > file. Both are *explicit caller selections* of an identity # for one invocation — the same trust model the FILE variable already had, since -# a caller could always point that at a file it wrote. Note the widened reach -# that follows from the rename: a non-`PAPERCLIP_` key can now be set from -# project/environment/routine env, not just agent scope. That is a downgrade -# vector, not an escalation one — it can only swap in a credential the setter -# already holds, never read the mounted one — but it does mean `gh` identity -# selection is only as tight as write access to those env scopes. +# a caller could always point that at a file it wrote. +# +# Dropping the `PAPERCLIP_` prefix would otherwise have widened who can set this +# key: environment/project/routine env are overlaid *after* agent-scope +# resolution, so the lowest-trust writer would win and could swap the identity +# `gh` runs as, or park whitespace here and fail every invocation with exit 64. +# The prefix used to prevent that for free. It is now prevented explicitly +# instead: AGENT_SCOPE_ONLY_ENV_KEYS in server/src/services/heartbeat.ts strips +# this key from environment, project and routine env, so only an agent-scoped +# secret binding can set it. Keep those two in sync — renaming here without +# renaming there silently reopens the hole. # # This is deliberately NOT GH_TOKEN: the override below must keep clobbering # GH_TOKEN unconditionally (BLO-13241), so GH_TOKEN cannot double as an input diff --git a/server/src/__tests__/heartbeat-project-env.test.ts b/server/src/__tests__/heartbeat-project-env.test.ts index 221a8a4506d2..0414700f7bb0 100644 --- a/server/src/__tests__/heartbeat-project-env.test.ts +++ b/server/src/__tests__/heartbeat-project-env.test.ts @@ -5,8 +5,10 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { buildSkillMentionHref } from "@paperclipai/shared"; import { LOW_TRUST_REVIEW_PRESET, + PUSH_CAPABILITY_ENV_KEYS, applyRunScopedMentionedSkillKeys, extractMentionedSkillIdsFromSources, + requiresPushCapabilityPreflight, resolveExecutionRunAdapterConfig, } from "../services/heartbeat.ts"; @@ -260,6 +262,164 @@ describe("resolveExecutionRunAdapterConfig", () => { expect(result.resolvedConfig.env).toHaveProperty("GH_SEAT_TOKEN_VALUE"); }); + // The rename above bought reachability at agent scope; on its own it would + // also have bought reachability at *every lower* scope, which is strictly + // worse than the PAPERCLIP_ name it replaced. Environment/project/routine env + // are overlaid AFTER agent resolution, so an unprotected key means the + // lowest-trust writer wins — and because the wrapper prefers this value over + // the mounted App token, that writer picks the identity every `gh` call runs + // as. Whitespace there fails them all with exit 64. These two tests pin the + // asymmetry: agent scope may set it, no lower scope may set or override it. + it("does not let environment, project, or routine env override an agent-scoped seat token", async () => { + const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ + config: { env: { GH_SEAT_TOKEN_VALUE: "agent-seat-token" } }, + secretKeys: new Set(), + manifest: [], + }); + const resolveEnvBindings = vi.fn(async (_companyId, env: Record) => ({ + env: Object.fromEntries( + Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"), + ), + secretKeys: new Set(), + manifest: [], + })); + + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { env: { GH_SEAT_TOKEN_VALUE: "agent-seat-token" } }, + environmentEnv: { GH_SEAT_TOKEN_VALUE: "environment-attacker", ENV_ONLY: "environment-only" }, + projectEnv: { GH_SEAT_TOKEN_VALUE: "project-attacker", PROJECT_ONLY: "project-only" }, + routineEnv: { GH_SEAT_TOKEN_VALUE: " ", ROUTINE_ONLY: "routine-only" }, + secretsSvc: { resolveAdapterConfigForRuntime, resolveEnvBindings } as any, + }); + + // The agent-scoped value survives all three overlays... + expect(result.resolvedConfig.env).toMatchObject({ GH_SEAT_TOKEN_VALUE: "agent-seat-token" }); + // ...and the lower scopes are otherwise unaffected, so this is a targeted + // filter and not an accidental drop of the whole overlay. + expect(result.resolvedConfig.env).toMatchObject({ + ENV_ONLY: "environment-only", + PROJECT_ONLY: "project-only", + ROUTINE_ONLY: "routine-only", + }); + // The key never even reaches the binding resolver for a lower scope, so a + // secret_ref planted there cannot be dereferenced as a side effect. + for (const call of resolveEnvBindings.mock.calls) { + expect(call[1]).not.toHaveProperty("GH_SEAT_TOKEN_VALUE"); + } + }); + + it("does not let a lower scope introduce a seat token the agent never had", async () => { + const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ + config: { env: { AGENT_ONLY: "agent-only" } }, + secretKeys: new Set(), + manifest: [], + }); + const resolveEnvBindings = vi.fn(async (_companyId, env: Record) => ({ + env: Object.fromEntries( + Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"), + ), + secretKeys: new Set(), + manifest: [], + })); + + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { env: { AGENT_ONLY: "agent-only" } }, + projectEnv: { GH_SEAT_TOKEN_VALUE: "project-attacker" }, + secretsSvc: { resolveAdapterConfigForRuntime, resolveEnvBindings } as any, + }); + + expect(result.resolvedConfig.env).not.toHaveProperty("GH_SEAT_TOKEN_VALUE"); + }); + + // Ally's finding: raw resolution preserving the key is necessary but not + // sufficient. requiresPushCapabilityPreflight-gated runs assert a push + // credential is configured BEFORE any of the above executes, and that + // contract listed only GH_TOKEN/GITHUB_TOKEN — so an agent bound exactly as + // BLO-18927 step 3 intends was rejected as push_write_credential_missing and + // the wrapper never ran. This exercises the real production contract by + // importing PUSH_CAPABILITY_ENV_KEYS rather than restating it, so the test + // cannot drift away from the constant it is guarding. + describe("push-capability preflight", () => { + const pushCapabilityBinding = { + keys: [...PUSH_CAPABILITY_ENV_KEYS], + consumerScopes: ["agent", "project"] as Array<"agent" | "project">, + reason: "push_write_credential_missing", + remediation: "test remediation", + }; + const stubSecretsSvc = () => ({ + resolveAdapterConfigForRuntime: vi.fn(async (_companyId, config: Record) => ({ + config: { ...config, env: { ...(config.env as Record) } }, + secretKeys: new Set(), + manifest: [], + })), + resolveEnvBindings: vi.fn(async () => ({ + env: {}, + secretKeys: new Set(), + manifest: [], + })), + }); + + it("gates on git-sensitive local adapters running the github-pr-workflow skill", () => { + expect(requiresPushCapabilityPreflight({ + adapterType: "opencode_local", + issueId: "issue-1", + explicitRunScopedSkillKeys: ["github-pr-workflow"], + })).toBe(true); + }); + + it("accepts an agent-scoped GH_SEAT_TOKEN_VALUE as a push credential", async () => { + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { + env: { + GH_SEAT_TOKEN_VALUE: { + type: "secret_ref", + // Must be a real UUID: isConfiguredEnvBindingValue validates the + // binding through envBindingSchema, and a malformed secretId + // makes it read as "not configured" rather than as an error. + secretId: "6f1c0c6e-6f2e-4a1e-9c2f-2b7d3a5e8c11", + version: "latest", + }, + }, + }, + requiredScopedEnvBinding: pushCapabilityBinding, + secretsSvc: stubSecretsSvc() as any, + }); + + expect(result.resolvedConfig.env).toHaveProperty("GH_SEAT_TOKEN_VALUE"); + }); + + it("still rejects a run with no push credential at any accepted key", async () => { + await expect(resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { env: { UNRELATED: "value" } }, + requiredScopedEnvBinding: pushCapabilityBinding, + secretsSvc: stubSecretsSvc() as any, + })).rejects.toThrow(/configuration incomplete/i); + }); + + // The seat key is agent-scope-only, so binding it at project scope must NOT + // satisfy the preflight — otherwise the gate would pass on a binding the + // strip above guarantees never arrives, dispatching a run that then fails + // with no credential at all. + it("does not accept a project-scoped seat token, which the overlay filter strips", async () => { + await expect(resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { env: {} }, + projectEnv: { GH_SEAT_TOKEN_VALUE: "project-seat-token" }, + requiredScopedEnvBinding: pushCapabilityBinding, + secretsSvc: stubSecretsSvc() as any, + })).rejects.toThrow(/configuration incomplete/i); + }); + }); + it("skips project env resolution when the project has no bindings", async () => { const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ config: { env: { AGENT_ONLY: "agent-only" } }, @@ -435,6 +595,41 @@ describe("resolveExecutionRunAdapterConfig", () => { }); }); + // GH_SEAT_TOKEN_VALUE carries a raw token but matches none of the name-shaped + // substrings the sensitive-key heuristic looks for, so without an explicit + // rule a low-trust run could inline the seat credential. Introduced with the + // key itself (BLO-18927) rather than left for later. + it("rejects an inline agent-scope-only seat token for low-trust runs", async () => { + await expect(resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + issueId: "issue-1", + executionRunConfig: { + env: { + GH_SEAT_TOKEN_VALUE: "inline-seat-token", + }, + }, + projectEnv: null, + trustPreset: { + kind: "low_trust_review", + preset: LOW_TRUST_REVIEW_PRESET, + boundary: { + mode: LOW_TRUST_REVIEW_PRESET, + companyId: "company-1", + issueIds: ["issue-1"], + }, + sourcePresets: {}, + }, + secretsSvc: { + resolveAdapterConfigForRuntime: vi.fn(), + resolveEnvBindings: vi.fn(), + } as any, + })).rejects.toMatchObject({ + status: 422, + details: { code: "low_trust_inline_sensitive_env_denied" }, + }); + }); + it("fails push-capability preflight when no GitHub write credential is bound at agent or project scope", async () => { await expect(resolveExecutionRunAdapterConfig({ companyId: "company-1", diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 7706e7c5c5b7..3e0a22b57251 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -592,7 +592,16 @@ const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_WAKE_REASON = "execution_review_part const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_CAUSE = "execution_review_participant_recovery"; const GITHUB_PR_WORKFLOW_SKILL_KEY = "paperclipai/bundled/software-development/github-pr-workflow"; const GITHUB_PR_WORKFLOW_SKILL_SLUG = "github-pr-workflow"; -const PUSH_CAPABILITY_ENV_KEYS = ["GH_TOKEN", "GITHUB_TOKEN"] as const; +// GH_SEAT_TOKEN_VALUE is a first-class member of this contract, not an +// afterthought: it is the *only* one of the three that can be delivered without +// mounting a credential into every Job pod (BLO-18927). Omitting it meant an +// agent bound correctly for the scoped path was still rejected as +// `push_write_credential_missing` before scripts/gh-token-wrapper.sh ever ran, +// which would have made the scoped binding unusable for exactly the git- +// sensitive local adapters this preflight guards. Widening the accepted set is +// safe: this gate asserts that *some* push credential is configured, it does +// not authorize anything. +export const PUSH_CAPABILITY_ENV_KEYS = ["GH_TOKEN", "GITHUB_TOKEN", "GH_SEAT_TOKEN_VALUE"] as const; // Keep this in sync with local adapters that require a git workspace before launch. const GIT_SENSITIVE_LOCAL_ADAPTER_TYPES = new Set([ "claude_local", @@ -1160,6 +1169,43 @@ function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record | null { + const record = parseObject(envValue); + const filtered = Object.fromEntries( + Object.entries(record).filter( + ([key]) => !isPaperclipRuntimeEnvKey(key) && !isAgentScopeOnlyEnvKey(key), + ), + ); + return Object.keys(filtered).length > 0 ? filtered : null; +} + function assertLowTrustEnvConfigAllowed(envValue: unknown, source: string) { const record = stripPaperclipRuntimeEnvBindings(envValue); if (!record) return; @@ -1170,7 +1216,13 @@ function assertLowTrustEnvConfigAllowed(envValue: unknown, source: string) { const isPlainBinding = typeof binding === "string" || (typeof binding === "object" && binding !== null && binding.type === "plain"); - if (isPlainBinding && LOW_TRUST_SENSITIVE_ENV_KEY_RE.test(key)) { + // Agent-scope-only keys hold a raw credential by construction, but do not + // match the name-shaped heuristic below (GH_SEAT_TOKEN_VALUE contains no + // "secret"/"auth"/"access_token" substring). Treat them as sensitive + // explicitly so a low-trust run cannot inline one; it must use a + // secret_ref. Safe to add with this PR because the key is new here — no + // existing config can be relying on the inline form. + if (isPlainBinding && (LOW_TRUST_SENSITIVE_ENV_KEY_RE.test(key) || isAgentScopeOnlyEnvKey(key))) { throw new HttpError(422, `Low-trust execution cannot use inline sensitive env value ${source}.${key}`, { code: "low_trust_inline_sensitive_env_denied", }); @@ -1202,9 +1254,9 @@ export async function resolveExecutionRunAdapterConfig(input: { }; }) { const executionRunConfig = stripPaperclipRuntimeEnvFromAdapterConfig(input.executionRunConfig); - const environmentEnv = stripPaperclipRuntimeEnvBindings(input.environmentEnv); - const projectEnv = stripPaperclipRuntimeEnvBindings(input.projectEnv); - const routineEnv = stripPaperclipRuntimeEnvBindings(input.routineEnv); + const environmentEnv = stripLowerScopeEnvBindings(input.environmentEnv); + const projectEnv = stripLowerScopeEnvBindings(input.projectEnv); + const routineEnv = stripLowerScopeEnvBindings(input.routineEnv); const lowTrustAllowedBindingIds = input.trustPreset?.kind === "low_trust_review" ? input.trustPreset.boundary.allowedSecretBindingIds ?? [] : undefined; @@ -18550,7 +18602,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) consumerScopes: ["agent", "project"], reason: "push_write_credential_missing", remediation: - "GitHub PR workflow requires GH_TOKEN or GITHUB_TOKEN bound at project or agent scope.", + "GitHub PR workflow requires GH_TOKEN or GITHUB_TOKEN bound at project or agent scope, or GH_SEAT_TOKEN_VALUE bound at agent scope.", } : undefined, }); From 29ccb00efa52eac02d1969601ee6ba784294985c Mon Sep 17 00:00:00 2001 From: "CTO (Paperclip agent)" Date: Sun, 2 Aug 2026 12:31:51 +0000 Subject: [PATCH 3/5] fix(heartbeat): close the issue-level adapter override route to the seat token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the still-present Important finding from Ally's review of #955 at head 83465d8. The previous fix filtered environment/project/routine env, which was the route I had reasoned about; it left a second route open one overlay earlier, and that one is worse because it lands *inside* what the resolver treats as agent scope rather than outside it. Chain: parseIssueAssigneeAdapterOverrides (:4831) accepts arbitrary adapterConfig keys from issue.assigneeAdapterOverrides, which any actor able to create or patch the issue can set. mergeModelProfileAdapterConfig (:3941) spreads it *last* over the agent config, and the result is passed to resolveExecutionRunAdapterConfig as executionRunConfig, where :1297 strips only PAPERCLIP_*. So an issue override could set GH_SEAT_TOKEN_VALUE and select the identity every `gh` invocation authenticates as. This is a regression this PR introduces, not a pre-existing hole: before the rename the key lived in the PAPERCLIP_ namespace, so the :1297 strip covered this route too. It is in scope for exactly that reason. The overlays are a *shallow* spread, so an overlay carrying `env` at all replaces the agent's `env` wholesale. That makes denial an exploit as much as substitution — parking whitespace in the key fails every `gh` invocation with exit 64, and simply supplying an unrelated `env` key drops the binding without ever naming it. withAgentScopedEnvProvenance therefore establishes a post-condition rather than filtering one input: after the merge, every AGENT_SCOPE_ONLY_ENV_KEY holds exactly the baseConfig value, and any the baseConfig lacks is absent. Both directions closed. Fixing it at resolveExecutionRunAdapterConfig instead would not work — by then provenance is gone and agent-set and issue-set values are indistinguishable. Scope note: this also ignores the key when it arrives via modelProfile.adapterConfig, which can be agent-provenanced (configSource "agent_runtime"). Deliberate and documented — the key resolves from the agent's primary config and nowhere else, so there is one place to audit. Verification: heartbeat-model-profile 11/11, heartbeat-project-env 24/24, tsc --noEmit clean. Mutation-checked — reverting the call to withAgentScopedEnvProvenance fails exactly the 4 new security assertions and no others; the fifth new test asserts unchanged overlay semantics for every other key and passes both ways by design. --- .../__tests__/heartbeat-model-profile.test.ts | 91 +++++++++++++++++++ server/src/services/heartbeat.ts | 46 +++++++++- 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-model-profile.test.ts b/server/src/__tests__/heartbeat-model-profile.test.ts index 5f1b31c24635..c5f8a40b9c96 100644 --- a/server/src/__tests__/heartbeat-model-profile.test.ts +++ b/server/src/__tests__/heartbeat-model-profile.test.ts @@ -151,3 +151,94 @@ describe("heartbeat model profile application", () => { expect(isConfigurationIncompleteFailedRun({ errorCode: "provider_quota" })).toBe(false); }); }); + +// The merged config is handed to resolveExecutionRunAdapterConfig as +// `executionRunConfig`, which treats it wholesale as agent scope and strips only +// `PAPERCLIP_*`. issue.assigneeAdapterOverrides.adapterConfig is overlaid into it +// last and accepts arbitrary keys, so without withAgentScopedEnvProvenance an +// issue override reaches agent scope — the boundary BLO-18927 exists to draw. +// GH_SEAT_TOKEN_VALUE selects the identity every `gh` invocation authenticates +// as, so both introducing and dropping it are exploits, and both are asserted. +describe("mergeModelProfileAdapterConfig agent-scope-only env boundary", () => { + const noProfile = { + requested: null, + requestedBy: null, + applied: null, + configSource: null, + fallbackReason: null, + adapterConfig: null, + } as const; + + it("does not let an issue adapter override introduce a seat token the agent never had", () => { + const merged = mergeModelProfileAdapterConfig({ + baseConfig: { env: { AGENT_ONLY: "agent-only" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { env: { GH_SEAT_TOKEN_VALUE: "issue-attacker" } }, + }); + + expect(merged.env).not.toHaveProperty("GH_SEAT_TOKEN_VALUE"); + }); + + it("does not let an issue adapter override replace an agent-scoped seat token", () => { + const merged = mergeModelProfileAdapterConfig({ + baseConfig: { env: { GH_SEAT_TOKEN_VALUE: "agent-seat-token" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { env: { GH_SEAT_TOKEN_VALUE: "issue-attacker" } }, + }); + + expect(merged.env).toMatchObject({ GH_SEAT_TOKEN_VALUE: "agent-seat-token" }); + }); + + // Whitespace in the key fails every `gh` invocation with exit 64, so denial is + // as much an exploit as substitution. The shallow overlay spread replaces the + // agent's `env` wholesale, which is how an override reaches this without ever + // naming the key. + it("does not let an issue adapter override drop or blank an agent-scoped seat token", () => { + const blanked = mergeModelProfileAdapterConfig({ + baseConfig: { env: { GH_SEAT_TOKEN_VALUE: "agent-seat-token" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { env: { GH_SEAT_TOKEN_VALUE: " " } }, + }); + const displaced = mergeModelProfileAdapterConfig({ + baseConfig: { env: { GH_SEAT_TOKEN_VALUE: "agent-seat-token" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { env: { UNRELATED: "issue-only" } }, + }); + + expect(blanked.env).toMatchObject({ GH_SEAT_TOKEN_VALUE: "agent-seat-token" }); + expect(displaced.env).toMatchObject({ GH_SEAT_TOKEN_VALUE: "agent-seat-token" }); + }); + + it("does not let a model profile overlay introduce a seat token", () => { + const merged = mergeModelProfileAdapterConfig({ + baseConfig: { env: { AGENT_ONLY: "agent-only" } }, + modelProfile: { + ...noProfile, + applied: "cheap", + configSource: "agent_runtime", + adapterConfig: { env: { GH_SEAT_TOKEN_VALUE: "profile-attacker" } }, + }, + issueAdapterConfig: null, + }); + + expect(merged.env).not.toHaveProperty("GH_SEAT_TOKEN_VALUE"); + }); + + // The boundary is scoped to AGENT_SCOPE_ONLY_ENV_KEYS; every other key keeps + // the pre-existing shallow-overlay semantics the tests above this rely on. + it("leaves non-agent-scope-only overlay semantics unchanged", () => { + const merged = mergeModelProfileAdapterConfig({ + baseConfig: { model: "primary", env: { SHARED: "agent", AGENT_ONLY: "agent-only" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { env: { SHARED: "issue" } }, + }); + const untouched = mergeModelProfileAdapterConfig({ + baseConfig: { model: "primary", env: { SHARED: "agent" } }, + modelProfile: { ...noProfile }, + issueAdapterConfig: { model: "issue-explicit" }, + }); + + expect(merged).toEqual({ model: "primary", env: { SHARED: "issue" } }); + expect(untouched).toEqual({ model: "issue-explicit", env: { SHARED: "agent" } }); + }); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 3e0a22b57251..9436a9b3766b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -1177,7 +1177,9 @@ function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record return Object.keys(filtered).length > 0 ? filtered : null; } +// Restores agent provenance for AGENT_SCOPE_ONLY_ENV_KEYS after the overlay +// spread in mergeModelProfileAdapterConfig, whose result is handed to +// resolveExecutionRunAdapterConfig as `executionRunConfig` — i.e. treated +// wholesale as agent scope, and filtered there only for `PAPERCLIP_*`. +// +// Two properties of that merge break the agent-only boundary without this: +// +// - `issueAdapterConfig` is issue.assigneeAdapterOverrides.adapterConfig. +// parseIssueAssigneeAdapterOverrides accepts arbitrary keys, and any actor +// able to create or patch the issue can set them. It is overlaid last. +// - the overlays are a *shallow* spread, so an overlay carrying `env` at all +// replaces the agent's `env` wholesale instead of merging into it. +// +// So an issue override could both introduce a seat token the agent never had +// (selecting the identity every `gh` invocation authenticates as) and drop one +// the agent did have. Post-condition established here: every +// AGENT_SCOPE_ONLY_ENV_KEY present in the merged env holds exactly the +// baseConfig value, and any the baseConfig lacks is absent. That closes both +// directions. +// +// Note this also ignores the key when it arrives via modelProfile.adapterConfig, +// which can be agent-provenanced (configSource "agent_runtime"). Deliberate: the +// key resolves from the agent's primary config and nowhere else, so there is one +// place to audit rather than one per profile. +function withAgentScopedEnvProvenance( + merged: Record, + baseConfig: Record, +): Record { + // Reference-equal means no overlay supplied `env`, so nothing was displaced. + if (merged.env === baseConfig.env) return merged; + const env = Object.fromEntries( + Object.entries(parseObject(merged.env)).filter(([key]) => !isAgentScopeOnlyEnvKey(key)), + ); + for (const [key, value] of Object.entries(parseObject(baseConfig.env))) { + if (isAgentScopeOnlyEnvKey(key)) env[key] = value; + } + return { ...merged, env }; +} + function assertLowTrustEnvConfigAllowed(envValue: unknown, source: string) { const record = stripPaperclipRuntimeEnvBindings(envValue); if (!record) return; @@ -3902,11 +3943,12 @@ export function mergeModelProfileAdapterConfig(input: { modelProfile: ModelProfileApplication; issueAdapterConfig: Record | null | undefined; }): Record { - return { + const merged = { ...input.baseConfig, ...(input.modelProfile.adapterConfig ?? {}), ...(input.issueAdapterConfig ?? {}), }; + return withAgentScopedEnvProvenance(merged, input.baseConfig); } function modelProfileRunMetadata( From a5ebf4c0de251003ef3a86654d26b303dfd64a2a Mon Sep 17 00:00:00 2001 From: CTO Date: Sun, 2 Aug 2026 14:01:35 +0000 Subject: [PATCH 4/5] docs(gh-token-wrapper): correct delivery-scope comment to agent-scope only Ally review on head 29ccb00e: the comment described GH_SEAT_TOKEN_VALUE as delivered by 'per-agent / per-project env bindings', but this PR's AGENT_SCOPE_ONLY_ENV_KEYS strips the key from project, environment and routine scope (heartbeat.ts:1298-1300). Project scope is no longer a delivery route, so the comment documented a path that no longer exists. Comment-only; no behavior change. --- scripts/gh-token-wrapper.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/gh-token-wrapper.sh b/scripts/gh-token-wrapper.sh index 8f4aeaab5ace..9864f9dce20a 100755 --- a/scripts/gh-token-wrapper.sh +++ b/scripts/gh-token-wrapper.sh @@ -21,12 +21,15 @@ TOKEN_FILE="${PAPERCLIP_GITHUB_TOKEN_FILE:-/paperclip/.secrets/github-token/toke REAL_GH="${GH_TOKEN_WRAPPER_REAL_GH:-/usr/bin/gh.real}" # GH_SEAT_TOKEN_VALUE carries a token *value* rather than a path, for -# credentials delivered by the scoped secret-binding path (per-agent / -# per-project env bindings) instead of by a mounted secret volume. It exists so -# a credential can be given to specific agents without mounting it into every -# agent pod: the k8s adapters propagate every main-container secret volume into -# every Job pod with no agent or tenant filter (BLO-18927, BLO-18970), so a -# volume-delivered secret is necessarily fleet-wide. +# credentials delivered by the scoped secret-binding path (agent-scoped env +# bindings only) instead of by a mounted secret volume. Project, environment and +# routine scope are NOT delivery routes for this key: AGENT_SCOPE_ONLY_ENV_KEYS +# (server/src/services/heartbeat.ts) strips it from all three, so that a +# lower-trust writer cannot select the identity every `gh` call runs as. +# It exists so a credential can be given to specific agents without mounting it +# into every agent pod: the k8s adapters propagate every main-container secret +# volume into every Job pod with no agent or tenant filter (BLO-18927, +# BLO-18970), so a volume-delivered secret is necessarily fleet-wide. # # The name deliberately does NOT start with `PAPERCLIP_`. Do not "fix" it for # consistency with the FILE variable below — the prefix is load-bearing in the From d522ef67c6b2776bd900fc5d02dc5316f9f3993b Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Mon, 3 Aug 2026 17:06:12 -0700 Subject: [PATCH 5/5] Fix remote GitHub seat token env translation --- .../__tests__/heartbeat-project-env.test.ts | 117 +++++++++++++++++- server/src/services/heartbeat.ts | 62 +++++++++- 2 files changed, 171 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/heartbeat-project-env.test.ts b/server/src/__tests__/heartbeat-project-env.test.ts index 0414700f7bb0..00af13ef03d8 100644 --- a/server/src/__tests__/heartbeat-project-env.test.ts +++ b/server/src/__tests__/heartbeat-project-env.test.ts @@ -10,6 +10,7 @@ import { extractMentionedSkillIdsFromSources, requiresPushCapabilityPreflight, resolveExecutionRunAdapterConfig, + translateGithubSeatTokenForExecutionTarget, } from "../services/heartbeat.ts"; describe("resolveExecutionRunAdapterConfig", () => { @@ -337,12 +338,11 @@ describe("resolveExecutionRunAdapterConfig", () => { // Ally's finding: raw resolution preserving the key is necessary but not // sufficient. requiresPushCapabilityPreflight-gated runs assert a push - // credential is configured BEFORE any of the above executes, and that - // contract listed only GH_TOKEN/GITHUB_TOKEN — so an agent bound exactly as - // BLO-18927 step 3 intends was rejected as push_write_credential_missing and - // the wrapper never ran. This exercises the real production contract by - // importing PUSH_CAPABILITY_ENV_KEYS rather than restating it, so the test - // cannot drift away from the constant it is guarding. + // credential is configured before dispatch, while remote execution targets + // still need a stock-gh-compatible command env after secret resolution. These + // tests exercise the real production contract by importing + // PUSH_CAPABILITY_ENV_KEYS rather than restating it, so the tests cannot drift + // away from the constant they guard. describe("push-capability preflight", () => { const pushCapabilityBinding = { keys: [...PUSH_CAPABILITY_ENV_KEYS], @@ -362,6 +362,28 @@ describe("resolveExecutionRunAdapterConfig", () => { manifest: [], })), }); + const remoteTarget = (transport: "sandbox" | "ssh") => + transport === "sandbox" + ? { + kind: "remote" as const, + transport, + remoteCwd: "/workspace", + } + : { + kind: "remote" as const, + transport, + remoteCwd: "/workspace", + spec: { + host: "devbox.example", + port: 22, + username: "paperclip", + remoteWorkspacePath: "/workspace", + privateKey: null, + knownHosts: null, + strictHostKeyChecking: false, + remoteCwd: "/workspace", + }, + }; it("gates on git-sensitive local adapters running the github-pr-workflow skill", () => { expect(requiresPushCapabilityPreflight({ @@ -394,6 +416,89 @@ describe("resolveExecutionRunAdapterConfig", () => { expect(result.resolvedConfig.env).toHaveProperty("GH_SEAT_TOKEN_VALUE"); }); + it.each(["sandbox", "ssh"] as const)( + "translates an agent-scoped GH_SEAT_TOKEN_VALUE into standard GitHub env for remote %s runs", + async (transport) => { + const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record) => ({ + config: { + ...config, + env: { + ...(config.env as Record), + GH_SEAT_TOKEN_VALUE: " ghu_remote_seat\n", + }, + }, + secretKeys: new Set(["GH_SEAT_TOKEN_VALUE"]), + manifest: [], + })); + + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { + env: { + GH_SEAT_TOKEN_VALUE: { + type: "secret_ref", + secretId: "6f1c0c6e-6f2e-4a1e-9c2f-2b7d3a5e8c11", + version: "latest", + }, + }, + }, + requiredScopedEnvBinding: pushCapabilityBinding, + secretsSvc: { + resolveAdapterConfigForRuntime, + resolveEnvBindings: vi.fn(async () => ({ + env: {}, + secretKeys: new Set(), + manifest: [], + })), + } as any, + }); + + const commandConfig = translateGithubSeatTokenForExecutionTarget({ + runtimeConfig: result.resolvedConfig, + executionTarget: remoteTarget(transport), + }); + + expect(commandConfig.env).toMatchObject({ + GH_SEAT_TOKEN_VALUE: " ghu_remote_seat\n", + GH_TOKEN: "ghu_remote_seat", + GITHUB_TOKEN: "ghu_remote_seat", + }); + }, + ); + + it("keeps GH_SEAT_TOKEN_VALUE local-only so the wrapper owns local translation", () => { + const runtimeConfig = { + env: { + GH_SEAT_TOKEN_VALUE: "ghu_local_seat", + }, + }; + + expect(translateGithubSeatTokenForExecutionTarget({ + runtimeConfig, + executionTarget: { kind: "local" }, + })).toBe(runtimeConfig); + }); + + it("uses seat-token precedence over preexisting standard GitHub env on remote targets", () => { + const commandConfig = translateGithubSeatTokenForExecutionTarget({ + runtimeConfig: { + env: { + GH_SEAT_TOKEN_VALUE: "ghu_agent_seat", + GH_TOKEN: "ghu_project_token", + GITHUB_TOKEN: "ghu_project_token", + }, + }, + executionTarget: remoteTarget("sandbox"), + }); + + expect(commandConfig.env).toMatchObject({ + GH_SEAT_TOKEN_VALUE: "ghu_agent_seat", + GH_TOKEN: "ghu_agent_seat", + GITHUB_TOKEN: "ghu_agent_seat", + }); + }); + it("still rejects a run with no push credential at any accepted key", async () => { await expect(resolveExecutionRunAdapterConfig({ companyId: "company-1", diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 1be7ca1f79bc..ce9136a7df3f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -329,6 +329,7 @@ import { type RuntimeStatusUpdate, type SessionCompactionPolicy, } from "@paperclipai/adapter-utils"; +import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import { readPaperclipSkillSyncPreference, UNMANAGED_BACKGROUND_TASK_LIVENESS_REASON, @@ -646,6 +647,9 @@ const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_WAKE_REASON = "execution_review_part const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_CAUSE = "execution_review_participant_recovery"; const GITHUB_PR_WORKFLOW_SKILL_KEY = "paperclipai/bundled/software-development/github-pr-workflow"; const GITHUB_PR_WORKFLOW_SKILL_SLUG = "github-pr-workflow"; +const GH_SEAT_TOKEN_ENV_KEY = "GH_SEAT_TOKEN_VALUE"; +const STANDARD_GITHUB_CREDENTIAL_ENV_KEYS = ["GH_TOKEN", "GITHUB_TOKEN"] as const; + // GH_SEAT_TOKEN_VALUE is a first-class member of this contract, not an // afterthought: it is the *only* one of the three that can be delivered without // mounting a credential into every Job pod (BLO-18927). Omitting it meant an @@ -655,7 +659,57 @@ const GITHUB_PR_WORKFLOW_SKILL_SLUG = "github-pr-workflow"; // sensitive local adapters this preflight guards. Widening the accepted set is // safe: this gate asserts that *some* push credential is configured, it does // not authorize anything. -export const PUSH_CAPABILITY_ENV_KEYS = ["GH_TOKEN", "GITHUB_TOKEN", "GH_SEAT_TOKEN_VALUE"] as const; +export const PUSH_CAPABILITY_ENV_KEYS = [ + ...STANDARD_GITHUB_CREDENTIAL_ENV_KEYS, + GH_SEAT_TOKEN_ENV_KEY, +] as const; + +export function translateGithubSeatTokenForExecutionTarget(input: { + runtimeConfig: Record; + executionTarget: AdapterExecutionTarget | null | undefined; +}): Record { + if (input.executionTarget?.kind !== "remote") return input.runtimeConfig; + const env = parseObject(input.runtimeConfig.env); + const rawSeatToken = env[GH_SEAT_TOKEN_ENV_KEY]; + if (typeof rawSeatToken !== "string") return input.runtimeConfig; + + const seatToken = rawSeatToken.trim(); + if (!seatToken) { + throw new ConfigurationIncompleteFailure( + "configuration incomplete: GH_SEAT_TOKEN_VALUE is set but resolves to an empty GitHub credential.", + { + configurationIncomplete: { + reason: "github_seat_token_empty", + requiredEnvKeys: [...PUSH_CAPABILITY_ENV_KEYS], + requiredScopes: ["agent"], + missingBindings: [], + }, + }, + ); + } + if (/\s/.test(seatToken)) { + throw new ConfigurationIncompleteFailure( + "configuration incomplete: GH_SEAT_TOKEN_VALUE contains embedded whitespace and cannot be translated safely.", + { + configurationIncomplete: { + reason: "github_seat_token_malformed", + requiredEnvKeys: [...PUSH_CAPABILITY_ENV_KEYS], + requiredScopes: ["agent"], + missingBindings: [], + }, + }, + ); + } + + return { + ...input.runtimeConfig, + env: { + ...env, + GH_TOKEN: seatToken, + GITHUB_TOKEN: seatToken, + }, + }; +} // Keep this in sync with local adapters that require a git workspace before launch. const GIT_SENSITIVE_LOCAL_ADAPTER_TYPES = new Set([ "claude_local", @@ -1301,7 +1355,7 @@ function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record