mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 20:44:16 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs get their environment from user/adapter/project/routine env bindings resolved by the server heartbeat, plus `PAPERCLIP_*` runtime vars (identity, wake, workspace, API access) injected by the harness > - The heartbeat stripped **every** `PAPERCLIP_`-prefixed binding before resolution, so legitimately user-named keys (e.g. cloud provider token bindings like `PAPERCLIP_CLOUD_PROD_PROVIDER_RAILWAY_*`) were silently dropped and never reached the run env > - At the same time, several adapters honored an explicitly configured `PAPERCLIP_API_KEY` over the harness-minted run token, which is exactly the one key config must never control > - This pull request replaces the blanket prefix strip with a precise three-rule policy: never accept `PAPERCLIP_API_KEY` from config, always let harness-assigned runtime vars win, and let every other `PAPERCLIP_*`-named user binding flow through > - The benefit is that user secrets with a `PAPERCLIP_`-style name work like any other binding, while runtime identity and API credentials stay fully harness-controlled ## Linked Issues or Issue Description **Bug description** (no public issue exists): - **What happened:** Env bindings whose key starts with `PAPERCLIP_` (e.g. a cloud provider token a user deliberately named `PAPERCLIP_CLOUD_PROD_PROVIDER_RAILWAY_TOKEN`) were silently stripped by the server before secret resolution, so the spawned agent never received them. No error, no access event — the variable just never appeared. - **Expected behavior:** A user-named `PAPERCLIP_*` binding should reach the run env unless the harness itself uses that key. Only `PAPERCLIP_API_KEY` should be categorically rejected, and harness-assigned runtime vars (`PAPERCLIP_RUN_ID`, `PAPERCLIP_AGENT_ID`, wake/workspace vars, …) should always win over config. - **Steps to reproduce:** Configure an agent/project env binding named `PAPERCLIP_<ANYTHING>` (plain or secret_ref), run a heartbeat, and inspect the spawned process env — the key is absent. - **Deployment mode:** local server, any local adapter. Related prior PRs (different, save-time/API-layer blanket-ban approach; this PR supersedes that direction with a runtime allow-except-reserved policy): Refs #8239, Refs #8439. ## What Changed - `server/src/services/heartbeat.ts`: the pre-resolution strip now removes only `PAPERCLIP_API_KEY` (hard denylist) instead of every `PAPERCLIP_`-prefixed binding; other `PAPERCLIP_*` keys flow into binding resolution. Low-trust inline-sensitive-env checks now also cover those keys. - `packages/adapter-utils/src/server-utils.ts`: new `isForbiddenConfigEnvKey()` helper; the shared `refreshPaperclipWorkspaceEnvForExecution` merge drops `PAPERCLIP_API_KEY` from config and keeps harness-assigned `PAPERCLIP_*` keys authoritative. - `packages/adapter-utils/src/acpx-engine/execute.ts`: removed the explicit-`PAPERCLIP_API_KEY`-from-config allowance; the run token (`authToken`) is now always applied; config `PAPERCLIP_API_KEY` is ignored. - All local adapters (`claude-local`, `codex-local`, `cursor-local`, `gemini-local`, `grok-local`, `opencode-local`, `pi-local`) plus `cursor-cloud`, `hermes`, and the server `process` adapter: removed `hasExplicitApiKey`-style allowances so the harness token always wins, and guarded the remaining unguarded env-merge loops (claude-local inline loop, process adapter) with the same policy. - Tests updated/added: heartbeat binding-strip test now asserts the three-rule policy; adapter-utils merge tests assert the `PAPERCLIP_API_KEY` ban and `PAPERCLIP_*` pass-through; acpx engine tests moved credential fixtures to `authToken` and assert config `PAPERCLIP_API_KEY` is ignored while other `PAPERCLIP_*` config keys forward and still bust the session fingerprint on rotation. ## Verification - `pnpm vitest run packages/adapter-utils/src/server-utils.test.ts packages/adapter-utils/src/acpx-engine/execute.test.ts` — 127 passed - `pnpm vitest run server/src/__tests__/heartbeat-project-env.test.ts server/src/__tests__/heartbeat-local-environment.test.ts server/src/__tests__/claude-local-execute.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/cursor-local-execute.test.ts server/src/__tests__/gemini-local-execute.test.ts` — 68 passed - Adapter package execute suites and the server tests touching API-key fixtures (`heartbeat-run-log`, `redaction`, `effective-run-config-fingerprints`, `agent-permissions-routes`) — green. Three pre-existing sandbox/SSH fixture failures reproduce identically on clean `master` on this host and are unrelated. - `pnpm --filter <pkg> typecheck` for server, adapter-utils, and all nine touched adapter packages — all pass. ## Risks - Behavioral change: a deployment that relied on configuring a static `PAPERCLIP_API_KEY` in adapter config env loses that override — by design; the harness-minted run token is now the only source. When no run token exists, no API key is injected at all. - `PAPERCLIP_*`-named user bindings now reach binding resolution and run envs; a key that collides with a harness runtime var is still discarded at merge time, so runtime identity/wake/workspace vars cannot be spoofed. - Low risk otherwise: no migrations, no API surface changes. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic Claude 5 family, Mythos-class tier), extended thinking enabled, agentic tool use (file edits, shell, test runner) via Claude Agent SDK. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
98 lines
3.9 KiB
TypeScript
98 lines
3.9 KiB
TypeScript
// Re-export everything from the shared adapter-utils/server-utils package.
|
|
// This file is kept as a convenience shim so existing in-tree
|
|
// imports (process/, http/, heartbeat.ts) don't need rewriting.
|
|
import type { ChildProcess } from "node:child_process";
|
|
import { logger } from "../middleware/logger.js";
|
|
import * as serverUtils from "@paperclipai/adapter-utils/server-utils";
|
|
export type { RunProcessResult } from "@paperclipai/adapter-utils/server-utils";
|
|
|
|
type BuildInvocationEnvForLogsOptions = {
|
|
runtimeEnv?: NodeJS.ProcessEnv | Record<string, string>;
|
|
includeRuntimeKeys?: string[];
|
|
resolvedCommand?: string | null;
|
|
resolvedCommandEnvKey?: string;
|
|
};
|
|
|
|
export const runningProcesses: Map<string, { child: ChildProcess; graceSec: number; processGroupId: number | null }> =
|
|
serverUtils.runningProcesses;
|
|
export const MAX_CAPTURE_BYTES = serverUtils.MAX_CAPTURE_BYTES;
|
|
export const MAX_EXCERPT_BYTES = serverUtils.MAX_EXCERPT_BYTES;
|
|
export const parseObject = serverUtils.parseObject;
|
|
export const asString = serverUtils.asString;
|
|
export const asNumber = serverUtils.asNumber;
|
|
export const asBoolean = serverUtils.asBoolean;
|
|
export const asStringArray = serverUtils.asStringArray;
|
|
export const parseJson = serverUtils.parseJson;
|
|
export const appendWithCap = serverUtils.appendWithCap;
|
|
export const appendWithByteCap = serverUtils.appendWithByteCap;
|
|
export const resolvePathValue = serverUtils.resolvePathValue;
|
|
export const renderTemplate = serverUtils.renderTemplate;
|
|
export const redactEnvForLogs = serverUtils.redactEnvForLogs;
|
|
export const buildPaperclipEnv = serverUtils.buildPaperclipEnv;
|
|
export const isPaperclipRuntimeEnvKey = serverUtils.isPaperclipRuntimeEnvKey;
|
|
export const isForbiddenConfigEnvKey = serverUtils.isForbiddenConfigEnvKey;
|
|
export const defaultPathForPlatform = serverUtils.defaultPathForPlatform;
|
|
export const ensurePathInEnv = serverUtils.ensurePathInEnv;
|
|
export const ensureAbsoluteDirectory = serverUtils.ensureAbsoluteDirectory;
|
|
export const ensureCommandResolvable = serverUtils.ensureCommandResolvable;
|
|
export const resolveCommandForLogs = serverUtils.resolveCommandForLogs;
|
|
|
|
export function buildInvocationEnvForLogs(
|
|
env: Record<string, string>,
|
|
options: BuildInvocationEnvForLogsOptions = {},
|
|
): Record<string, string> {
|
|
const maybeBuildInvocationEnvForLogs = (
|
|
serverUtils as typeof serverUtils & {
|
|
buildInvocationEnvForLogs?: (
|
|
env: Record<string, string>,
|
|
options?: BuildInvocationEnvForLogsOptions,
|
|
) => Record<string, string>;
|
|
}
|
|
).buildInvocationEnvForLogs;
|
|
|
|
if (typeof maybeBuildInvocationEnvForLogs === "function") {
|
|
return maybeBuildInvocationEnvForLogs(env, options);
|
|
}
|
|
|
|
const merged: Record<string, string> = { ...env };
|
|
const runtimeEnv = options.runtimeEnv ?? {};
|
|
|
|
for (const key of options.includeRuntimeKeys ?? []) {
|
|
if (key in merged) continue;
|
|
const value = runtimeEnv[key];
|
|
if (typeof value !== "string" || value.length === 0) continue;
|
|
merged[key] = value;
|
|
}
|
|
|
|
const resolvedCommand = options.resolvedCommand?.trim();
|
|
if (resolvedCommand) {
|
|
merged[options.resolvedCommandEnvKey ?? "PAPERCLIP_RESOLVED_COMMAND"] =
|
|
serverUtils.redactCommandTextForLogs(resolvedCommand);
|
|
}
|
|
|
|
return redactEnvForLogs(merged);
|
|
}
|
|
|
|
// Re-export runChildProcess with the server's pino logger wired in.
|
|
import type { RunProcessResult } from "@paperclipai/adapter-utils/server-utils";
|
|
const _runChildProcess = serverUtils.runChildProcess;
|
|
|
|
export async function runChildProcess(
|
|
runId: string,
|
|
command: string,
|
|
args: string[],
|
|
opts: {
|
|
cwd: string;
|
|
env: Record<string, string>;
|
|
timeoutSec: number;
|
|
graceSec: number;
|
|
onLog: (stream: "stdout" | "stderr", chunk: string) => Promise<void>;
|
|
onSpawn?: (meta: { pid: number; processGroupId: number | null; startedAt: string }) => Promise<void>;
|
|
},
|
|
): Promise<RunProcessResult> {
|
|
return _runChildProcess(runId, command, args, {
|
|
...opts,
|
|
onLogError: (err, id, msg) => logger.warn({ err, runId: id }, msg),
|
|
});
|
|
}
|