fix(cli): restore test-drive credential inputs (#12898)
## 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 <value>` 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 <value>` 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 <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
70c9ca7410
commit
3da58b185e
|
|
@ -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";
|
||||
|
|
|
|||
|
|
@ -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<string | un
|
|||
return redacted;
|
||||
}
|
||||
|
||||
export function redactTestDriveArgv(
|
||||
apiKey: string | undefined,
|
||||
argv: string[] = process.argv,
|
||||
): void {
|
||||
if (!apiKey) return;
|
||||
for (let index = 0; index < argv.length; index += 1) {
|
||||
if (argv[index] === "--api-key" && argv[index + 1] === apiKey) {
|
||||
argv[index + 1] = "[REDACTED]";
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (argv[index] === `--api-key=${apiKey}`) {
|
||||
argv[index] = "--api-key=[REDACTED]";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveTestDriveDataDir(dataDir?: string): string {
|
||||
const explicit = dataDir?.trim();
|
||||
if (explicit) {
|
||||
|
|
@ -206,6 +224,10 @@ export function resolveTestDriveBootstrap(
|
|||
options: TestDriveOptions,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): ResolvedTestDriveBootstrap {
|
||||
if (options.apiKey !== undefined && options.apiKeyEnv !== undefined) {
|
||||
throw new Error("--api-key and --api-key-env are mutually exclusive.");
|
||||
}
|
||||
|
||||
const harness = options.harness ?? "claude";
|
||||
const definition = HARNESS_DEFINITIONS[harness];
|
||||
if (!definition) {
|
||||
|
|
@ -234,10 +256,10 @@ export function resolveTestDriveBootstrap(
|
|||
if (options.apiKeyEnv !== undefined && !/^[A-Za-z_][A-Za-z0-9_]*$/.test(sourceEnvName)) {
|
||||
throw new Error("--api-key-env must name a valid environment variable.");
|
||||
}
|
||||
const credential = env[sourceEnvName];
|
||||
const credential = options.apiKey ?? env[sourceEnvName];
|
||||
if (!credential || credential.trim().length === 0) {
|
||||
throw new Error(
|
||||
`No credential found. Set ${sourceEnvName} or pass --api-key-env <variable>.`,
|
||||
`No credential found. Set ${sourceEnvName}, pass --api-key-env <variable>, or pass --api-key <value>.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -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<void> {
|
||||
// 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 <model-id>", "Initial agent model")
|
||||
.option("--api-key-env <variable>", "Read the provider key from an environment variable")
|
||||
.addOption(
|
||||
new Option("--api-key-env <variable>", "Read the provider key from an environment variable")
|
||||
.conflicts("apiKey"),
|
||||
)
|
||||
.addOption(
|
||||
new Option("--api-key <value>", "Provider API key")
|
||||
.conflicts("apiKeyEnv"),
|
||||
)
|
||||
.option("--no-browser", "Do not open the initialized instance in a browser")
|
||||
.action(async (options: TestDriveOptions) => {
|
||||
await testDriveCommand(options);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
23
doc/CLI.md
23
doc/CLI.md
|
|
@ -169,7 +169,7 @@ npx paperclipai test-drive \
|
|||
[--agent-name <name>] \
|
||||
[--harness <claude|codex|opencode>] \
|
||||
[--model <model-id>] \
|
||||
[--api-key-env <variable>] \
|
||||
[--api-key-env <variable> | --api-key <value>] \
|
||||
[--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
|
||||
|
|
|
|||
|
|
@ -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 <value>` 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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue