From 3da58b185e4b383ee0a9dfa3d1dc0bb090d0be37 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 5 Sep 2026 10:48:17 -0500 Subject: [PATCH] fix(cli): restore test-drive credential inputs (#12898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The CLI provides a test-drive command for a ready local test instance. > - That command must accept a provider credential before it creates the first agent. > - The command did not accept a literal key and could lose an exported key during server startup. > - This pull request accepts both credential paths and captures the CLI environment before startup. > - The benefit is a reliable one-command test drive from an existing shell. ## Linked Issues or Issue Description Refs #12894 **What happened?** `paperclipai test-drive --api-key ` failed because the option did not exist. An exported canonical provider variable could also become unavailable before the post-listen bootstrap read it. **Expected behavior** The command must accept a literal key when the operator requests it. The command must also use a provider variable that was present when the CLI started. **Steps to reproduce** 1. Export `ANTHROPIC_API_KEY` in the shell. 2. Run `pnpm paperclipai test-drive`. 3. Observe that bootstrap can report that no credential exists. 4. Run `pnpm paperclipai test-drive --api-key test-value`. 5. Observe that Commander reports an unknown option on the prior implementation. **Paperclip version or commit** The problem exists on `master` after #12894. **Deployment mode** Local development with `pnpm` and the embedded database. ## What Changed - Add the `--api-key ` test-drive option. - Keep `--api-key` and `--api-key-env` mutually exclusive. - Capture provider variables before in-process server startup changes the process environment. - Redact literal and environment-backed credentials from Paperclip errors, including custom environment-variable names with surrounding whitespace. - Scrub split and joined literal-key forms from the JavaScript `process.argv` view before telemetry, diagnostics, API work, or server startup. - Warn about process argument and shell history exposure without printing the key. - Add tests for literal keys, option conflicts, environment snapshots, argv handling, and error redaction. - Update the CLI and development documentation, including the remaining external argv exposure tradeoff. ## Verification - `pnpm exec vitest run cli/src/__tests__/test-drive.test.ts` passed 32 tests. - `pnpm -r typecheck` passed. - `pnpm build` passed. - A live literal-key smoke test created one company and one CEO agent without a provider call. - A live exported-variable smoke test created the same clean instance without credential flags. - `pnpm test:run` completed locally with 5,870 passing tests and 19 host-dependent failures in six unrelated suites. The failures came from macOS `/tmp` aliases, exhausted test ports, and existing workspace-runtime fixture assumptions. - GitHub CI passed the full build, typecheck, canary dry run, general tests, serialized server tests, and e2e matrix on clean Linux runners. - Greptile rated the exact latest head 5/5, Superagent passed, and all review threads are resolved. ## Risks - A raw value passed through `--api-key` can appear in operating-system process listings, shell history, or parent-wrapper output before Paperclip can scrub its own JavaScript argv view. The command warns about this risk and Paperclip does not print the value. - The environment snapshot contains the process environment only in memory for the life of the foreground command. - This change adds no schema migration and no REST endpoint. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex with GPT-5. The exact deployment version and context window are not exposed to the agent. Extended reasoning, tool use, code execution, and GitHub access were enabled. ## 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 - [x] 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 --------- Co-authored-by: Paperclip --- cli/src/__tests__/test-drive.test.ts | 94 +++++++++++++++++++++++++++- cli/src/commands/test-drive.ts | 53 ++++++++++++++-- cli/src/index.ts | 2 + doc/CLI.md | 23 ++++--- doc/DEVELOPING.md | 8 ++- 5 files changed, 160 insertions(+), 20 deletions(-) diff --git a/cli/src/__tests__/test-drive.test.ts b/cli/src/__tests__/test-drive.test.ts index 63ba3017b8..3f89a14b96 100644 --- a/cli/src/__tests__/test-drive.test.ts +++ b/cli/src/__tests__/test-drive.test.ts @@ -8,6 +8,7 @@ import { bootstrapTestDrive, prepareTestDriveEnvironment, reconcileTestDriveWorktreeExecution, + redactTestDriveArgv, redactTestDriveText, resolveTestDriveBootstrap, resolveTestDriveDataDir, @@ -239,14 +240,41 @@ describe("test-drive bootstrap validation", () => { expect(resolved.credentialTarget).toBe("OPENROUTER_API_KEY"); }); + it("accepts a literal key and gives it precedence over canonical environment lookup", () => { + const resolved = resolveTestDriveBootstrap( + { apiKey: "literal-secret" }, + { ANTHROPIC_API_KEY: "environment-secret" }, + ); + expect(resolved.credential).toBe("literal-secret"); + expect(resolved.credentialSource).toBe("--api-key"); + }); + + it("rejects mutually exclusive key inputs", () => { + expect(() => resolveTestDriveBootstrap({ + apiKey: "literal-secret", + apiKeyEnv: "ANTHROPIC_API_KEY", + }, { ANTHROPIC_API_KEY: "environment-secret" })).toThrow(/mutually exclusive/); + }); + it("rejects invalid key variable names and redacts credentials", () => { expect(() => resolveTestDriveBootstrap({ apiKeyEnv: "NOT-A-VALID-NAME", }, { ANTHROPIC_API_KEY: "env-secret" })).toThrow(/valid environment variable/); expect(redactTestDriveText( - "custom-secret and env-secret must never appear", - ["custom-secret", "env-secret"], - )).toBe("[REDACTED] and [REDACTED] must never appear"); + "literal-secret, custom-secret, and env-secret must never appear", + ["literal-secret", "custom-secret", "env-secret"], + )).toBe("[REDACTED], [REDACTED], and [REDACTED] must never appear"); + }); + + it("removes literal keys from the JavaScript argv view", () => { + const splitArgv = ["node", "paperclipai", "test-drive", "--api-key", "literal-secret"]; + const joinedArgv = ["node", "paperclipai", "test-drive", "--api-key=literal-secret"]; + + redactTestDriveArgv("literal-secret", splitArgv); + redactTestDriveArgv("literal-secret", joinedArgv); + + expect(splitArgv).toEqual(["node", "paperclipai", "test-drive", "--api-key", "[REDACTED]"]); + expect(joinedArgv).toEqual(["node", "paperclipai", "test-drive", "--api-key=[REDACTED]"]); }); }); @@ -491,6 +519,66 @@ describe("test-drive foreground lifecycle", () => { expect(runOptions?.skipServiceManagerCheck).toBe(false); }); + it("uses the credential snapshot captured before downstream server initialization", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-test-drive-credential-")); + cleanupDirectories.push(root); + process.env.PAPERCLIP_HOME = root; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + process.env.PAPERCLIP_IN_WORKTREE = "false"; + process.env.ANTHROPIC_API_KEY = "upstream-secret"; + const { api, calls } = freshBootstrapApi(); + + await testDriveCommand({ browser: false }, { + run: async (options) => { + delete process.env.ANTHROPIC_API_KEY; + await options.afterStart?.(server); + }, + createApi: () => api, + openBrowser: vi.fn(async () => true), + }); + + const secretValueCall = calls.find((call) => call.path.endsWith("/me/user-secrets")); + expect(secretValueCall?.body).toEqual({ + definitionKey: "ANTHROPIC_API_KEY", + value: "upstream-secret", + }); + }); + + it("redacts a literal key from downstream errors", async () => { + process.env.PAPERCLIP_HOME = "/tmp/test-drive-redaction"; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + process.env.PAPERCLIP_IN_WORKTREE = "false"; + + await expect(testDriveCommand({ + apiKey: "literal-secret", + browser: false, + }, { + run: async () => { + throw new Error("downstream rejected literal-secret"); + }, + createApi: () => freshBootstrapApi().api, + openBrowser: vi.fn(async () => true), + })).rejects.toThrow("downstream rejected [REDACTED]"); + }); + + it("redacts a custom environment key when its option name has whitespace", async () => { + process.env.PAPERCLIP_HOME = "/tmp/test-drive-custom-env-redaction"; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + process.env.PAPERCLIP_IN_WORKTREE = "false"; + process.env.CUSTOM_TEST_DRIVE_KEY = "custom-secret"; + + await expect(testDriveCommand({ + apiKeyEnv: " CUSTOM_TEST_DRIVE_KEY ", + browser: false, + }, { + run: async () => { + throw new Error("downstream rejected custom-secret"); + }, + createApi: () => freshBootstrapApi().api, + openBrowser: vi.fn(async () => true), + })).rejects.toThrow("downstream rejected [REDACTED]"); + }); + it("honors --no-browser after successful initialization", async () => { process.env.PAPERCLIP_HOME = "/tmp/test-drive-no-browser"; process.env.PAPERCLIP_INSTANCE_ID = "default"; diff --git a/cli/src/commands/test-drive.ts b/cli/src/commands/test-drive.ts index eb6b87bc3a..a1b1f57690 100644 --- a/cli/src/commands/test-drive.ts +++ b/cli/src/commands/test-drive.ts @@ -28,6 +28,7 @@ export interface TestDriveOptions { harness?: TestDriveHarness; model?: string; apiKeyEnv?: string; + apiKey?: string; browser?: boolean; } @@ -113,6 +114,23 @@ export function redactTestDriveText(text: string, credentials: Array.`, + `No credential found. Set ${sourceEnvName}, pass --api-key-env , or pass --api-key .`, ); } @@ -247,7 +269,7 @@ export function resolveTestDriveBootstrap( agentName, ...(model ? { model } : {}), credential, - credentialSource: sourceEnvName, + credentialSource: options.apiKey !== undefined ? "--api-key" : sourceEnvName, }; } @@ -385,16 +407,27 @@ export async function testDriveCommand( openBrowser: openUrl, }, ): Promise { + // Commander has already copied the value into options. Remove it from the + // JavaScript argv view before logging, telemetry, diagnostics, or startup. + redactTestDriveArgv(options.apiKey); const dataDir = path.resolve(process.env.PAPERCLIP_HOME ?? resolveTestDriveDataDir(options.dataDir)); const linkedWorktree = process.env.PAPERCLIP_IN_WORKTREE === "true"; const instanceId = process.env.PAPERCLIP_INSTANCE_ID ?? "default"; + // Resolve environment-backed credentials against the CLI environment as it + // exists before server startup. In-process server initialization must not + // change which credential the post-listen bootstrap observes. + const bootstrapEnv = { ...process.env }; const possibleCredentials = [ - options.apiKeyEnv ? process.env[options.apiKeyEnv] : undefined, - process.env[HARNESS_DEFINITIONS[options.harness ?? "claude"].credentialTarget], + options.apiKey, + options.apiKeyEnv ? bootstrapEnv[options.apiKeyEnv.trim()] : undefined, + bootstrapEnv[HARNESS_DEFINITIONS[options.harness ?? "claude"].credentialTarget], ]; p.log.message(pc.dim(`Data directory: ${dataDir}`)); p.log.message(pc.dim("The data directory is retained when Paperclip exits.")); + if (options.apiKey !== undefined) { + p.log.warn("A key passed with --api-key may be visible in process arguments and shell history."); + } try { await dependencies.run({ @@ -413,6 +446,7 @@ export async function testDriveCommand( options, linkedWorktree, instanceId, + env: bootstrapEnv, }); if (result.reused) { p.log.message( @@ -458,7 +492,14 @@ export function registerTestDriveCommand(program: Command): void { .default("claude"), ) .option("--model ", "Initial agent model") - .option("--api-key-env ", "Read the provider key from an environment variable") + .addOption( + new Option("--api-key-env ", "Read the provider key from an environment variable") + .conflicts("apiKey"), + ) + .addOption( + new Option("--api-key ", "Provider API key") + .conflicts("apiKeyEnv"), + ) .option("--no-browser", "Do not open the initialized instance in a browser") .action(async (options: TestDriveOptions) => { await testDriveCommand(options); diff --git a/cli/src/index.ts b/cli/src/index.ts index 156abd8246..2aab16c038 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -53,6 +53,7 @@ import { registerConnectionIntentCommands } from "./commands/client/connections. import { assertTestDriveDatabaseIsolation, prepareTestDriveEnvironment, + redactTestDriveArgv, registerTestDriveCommand, type TestDriveOptions, } from "./commands/test-drive.js"; @@ -102,6 +103,7 @@ program.hook("preAction", async (_thisCommand, actionCommand) => { const options = actionCommand.optsWithGlobals() as DataDirOptionLike & TestDriveOptions; let dataDirOptions: DataDirOptionLike = options; if (actionCommand.name() === "test-drive") { + redactTestDriveArgv(options.apiKey); const prepared = await prepareTestDriveEnvironment({ dataDir: options.dataDir, apiKeyEnv: options.apiKeyEnv, diff --git a/doc/CLI.md b/doc/CLI.md index b137483feb..f9a6e52a3c 100644 --- a/doc/CLI.md +++ b/doc/CLI.md @@ -169,7 +169,7 @@ npx paperclipai test-drive \ [--agent-name ] \ [--harness ] \ [--model ] \ - [--api-key-env ] \ + [--api-key-env | --api-key ] \ [--no-browser] ``` @@ -203,9 +203,10 @@ OPENROUTER_API_KEY=... npx paperclipai test-drive \ --model openrouter/anthropic/claude-sonnet-4.5 ``` -Credentials come from the variable named by `--api-key-env`, or from the -harness's canonical environment variable shown in the table. A custom source -variable is still stored and projected under the canonical target variable: +Credentials come from `--api-key`, the variable named by `--api-key-env`, or +the harness's canonical environment variable shown in the table. `--api-key` +and `--api-key-env` are mutually exclusive. A custom source variable is still +stored and projected under the canonical target variable: ```sh MY_ROUTER_KEY=... npx paperclipai test-drive \ @@ -215,11 +216,15 @@ MY_ROUTER_KEY=... npx paperclipai test-drive \ ``` Credentials are stored through Paperclip's user-secret reference path and are -redacted from command output. The command does not accept provider credentials -in command-line arguments because process listings and shell history can expose -them. Provider connectivity, local harness installation, credential validity, -and model availability are intentionally checked only when the agent first -runs. +redacted from Paperclip command output. Paperclip does not print `--api-key`, +and it removes the value from its JavaScript argument view immediately after +Commander parses it. Paperclip does not put the raw argument list in telemetry, +API metadata, or diagnostics. Command wrappers, operating-system process +listings, and shell history can still expose values passed in arguments. This +is an explicit tradeoff for the local test-drive workflow. Prefer an exported +canonical variable or `--api-key-env` when that matters. Provider connectivity, +local harness installation, credential validity, and model availability are +intentionally checked only when the agent first runs. When invoked inside a linked Git worktree, the command ignores inherited `PAPERCLIP_IN_WORKTREE` state, launches in worktree mode, and verifies **Run diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 6bf2b7f5e4..cab7762fce 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -334,8 +334,12 @@ selects the first available loopback port at or above `3100`. Claude uses `ANTHROPIC_API_KEY`; Codex uses `OPENAI_API_KEY`; OpenCode uses `OPENROUTER_API_KEY` and requires an `openrouter/...` model. `--api-key-env` can name a different source variable while the agent still receives the -canonical variable. Provider keys must come from environment variables because -command-line arguments can appear in process listings and shell history. In a +canonical variable. `--api-key ` is also supported and is mutually +exclusive with `--api-key-env`; Paperclip redacts it from its own output, but +also removes it from the JavaScript argument view before telemetry, diagnostics, +or server startup. Wrappers, operating-system process listings, and shell +history may still expose argument values. This is an explicit local test-drive +tradeoff; use an environment-backed input when that exposure is not acceptable. In a linked Git worktree the command sets worktree runtime mode and safely arms **Run tasks in this worktree** for the current isolated instance. Primary checkouts and non-Git directories leave that experimental setting unchanged.