From 5802762463ce03e09dc59ce6ffa3b807079603a0 Mon Sep 17 00:00:00 2001 From: Rick Gao Date: Thu, 28 May 2026 17:44:48 -0700 Subject: [PATCH 1/3] fix(config): mask wandb_api_key in `config set` output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `config set wandb_api_key ` echoed the full secret to stdout, leaking it into terminal scrollback, CI logs, and tool transcripts whenever the install/migration flow ran. `config show` and `status` already masked to first-4-chars + ellipsis; only `set` was missed when `wandb_api_key` was added as a writable key. Extracted a single `maskSecret` helper used by all three call sites so the mask format lives in one place. `config get wandb_api_key` is intentionally left un-masked — it's the programmatic retrieval path (eg. the migration in the weave-install skill). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cli.ts | 17 +++- tests/config-set-masks-secrets.test.ts | 119 +++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 tests/config-set-masks-secrets.test.ts diff --git a/src/cli.ts b/src/cli.ts index 0edb5eb..8038977 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -186,6 +186,14 @@ async function cmdInstall(force: boolean, nonInteractive: boolean): Promise { const action = args[0]; @@ -211,7 +219,7 @@ async function cmdConfig(args: string[]): Promise { : settings.wandb_api_key ? 'settings.json' : 'not set'; - const apiKeyDisplay = effectiveApiKey ? `${effectiveApiKey.slice(0, 4)}… [${apiKeySource}]` : `(not set)`; + const apiKeyDisplay = effectiveApiKey ? `${maskSecret(effectiveApiKey)} [${apiKeySource}]` : `(not set)`; console.log('Current configuration:'); console.log(` log_file: ${settings.log_file}`); @@ -289,7 +297,10 @@ async function cmdConfig(args: string[]): Promise { const coerced = key === 'debug' ? value === 'true' : value; (settings as unknown as Record)[key] = coerced; saveSettings(settings); - console.log(`✓ Set ${key} = ${value}`); + const displayValue = key === 'wandb_api_key' && typeof coerced === 'string' + ? maskSecret(coerced) + : coerced; + console.log(`✓ Set ${key} = ${displayValue}`); return; } @@ -345,7 +356,7 @@ async function cmdStatus(): Promise { const effectiveApiKey = process.env['WANDB_API_KEY'] ?? settings.wandb_api_key ?? null; if (effectiveApiKey) { const apiKeySource = process.env['WANDB_API_KEY'] ? 'WANDB_API_KEY env var' : 'settings.json'; - console.log(`✓ W&B API key: ${effectiveApiKey.slice(0, 4)}… (from ${apiKeySource})`); + console.log(`✓ W&B API key: ${maskSecret(effectiveApiKey)} (from ${apiKeySource})`); } else { console.log('✗ W&B API key: not configured'); console.log(' Run: weave-claude-code config set wandb_api_key '); diff --git a/tests/config-set-masks-secrets.test.ts b/tests/config-set-masks-secrets.test.ts new file mode 100644 index 0000000..d19e320 --- /dev/null +++ b/tests/config-set-masks-secrets.test.ts @@ -0,0 +1,119 @@ +// SPDX-FileCopyrightText: 2026 CoreWeave, Inc. +// SPDX-License-Identifier: MIT +// SPDX-PackageName: weave-claude-code + +// `weave-claude-code config set wandb_api_key ` echoes the value to +// stdout on success. `config show` masks the same value to first-4-chars + +// ellipsis, but `set` had no equivalent masking — the literal key leaked into +// terminal scrollback, CI logs, and tool transcripts whenever the install +// flow ran. These tests pin the masking contract for `set` and guard the +// existing non-masking behavior of non-sensitive keys. + +import { test, suite, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(HERE, '..'); +const CLI = path.join(REPO_ROOT, 'src', 'cli.ts'); + +// A distinctive secret so a substring match on stdout is unambiguous. +const SECRET = 'wandb_v1_SUPERSECRETvalueDoNotLeak0123456789'; + +interface Workspace { + home: string; + settingsFile: string; +} + +function newWorkspace(label: string): Workspace { + const home = fs.mkdtempSync(`/tmp/wcp-cfgset-${label}-`); + const configDir = path.join(home, '.weave-claude-code'); + fs.mkdirSync(path.join(configDir, 'logs'), { recursive: true }); + const settingsFile = path.join(configDir, 'settings.json'); + fs.writeFileSync( + settingsFile, + JSON.stringify({ + log_file: path.join(configDir, 'logs', 'daemon.log'), + daemon_socket: path.join(configDir, 'daemon.sock'), + weave_project: null, + wandb_api_key: null, + debug: false, + installed_at: '2026-01-01T00:00:00Z', + version: '0.0.0-test', + }), + ); + return { home, settingsFile }; +} + +function runCli(home: string, args: string[]): Promise<{ stdout: string; stderr: string; code: number | null }> { + return new Promise((resolve, reject) => { + // Scrub WANDB_API_KEY/WEAVE_PROJECT from the parent env so they can't + // shadow what the CLI reads back from the per-test settings.json. + const env = { ...process.env, HOME: home }; + delete env.WANDB_API_KEY; + delete env.WEAVE_PROJECT; + const child = spawn( + process.execPath, + ['--import', 'tsx', CLI, ...args], + { cwd: REPO_ROOT, env }, + ); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (b) => { stdout += b.toString(); }); + child.stderr.on('data', (b) => { stderr += b.toString(); }); + child.on('error', reject); + child.on('exit', (code) => resolve({ stdout, stderr, code })); + }); +} + +let workspaces: string[] = []; +beforeEach(() => { workspaces = []; }); +afterEach(() => { + for (const w of workspaces) fs.rmSync(w, { recursive: true, force: true }); +}); + +function workspace(label: string): Workspace { + const w = newWorkspace(label); + workspaces.push(w.home); + return w; +} + +suite('config set masks sensitive values in stdout', () => { + test('wandb_api_key: stdout does not contain the full secret', async () => { + const w = workspace('mask-stdout'); + const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); + assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); + assert.equal( + r.stdout.includes(SECRET), + false, + `stdout leaked the full secret:\n${r.stdout}`, + ); + }); + + test('wandb_api_key: stdout shows the masked prefix (first 4 chars + ellipsis)', async () => { + const w = workspace('mask-prefix'); + const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); + assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); + // Use the same shape `config show` already prints: `wand…` + assert.match(r.stdout, /wand…/, `stdout should contain masked prefix; got:\n${r.stdout}`); + }); + + test('wandb_api_key: full secret is still persisted to settings.json', async () => { + const w = workspace('mask-persist'); + const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); + assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); + const saved = JSON.parse(fs.readFileSync(w.settingsFile, 'utf8')); + assert.equal(saved.wandb_api_key, SECRET, 'on-disk value must be the full secret'); + }); + + test('weave_project: non-sensitive value is still echoed in full', async () => { + const w = workspace('echo-project'); + const project = 'my-entity/my-project'; + const r = await runCli(w.home, ['config', 'set', 'weave_project', project]); + assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); + assert.match(r.stdout, new RegExp(project.replace('/', '\\/')), `stdout should echo full project; got:\n${r.stdout}`); + }); +}); From 23ca623ad638fd33ed13f3f292405fbff99a6373 Mon Sep 17 00:00:00 2001 From: Rick Gao Date: Thu, 28 May 2026 18:59:23 -0700 Subject: [PATCH 2/3] test: consolidate config-set mask tests, trim comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tests of the same `config set` call (three on wandb_api_key, one on weave_project) collapse into one walking both branches of the mask-or-not decision: one CLI spawn for the secret path, one for the plain path. Also drops the maskSecret docstring (name is self-evident) and the test file's seven-line header in favor of a two-line "what + why". 9/9 passing (was 12/12 — the four merged into one and the suite wrapper went away). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cli.ts | 4 - tests/config-set-masks-secrets.test.ts | 120 +++++++------------------ 2 files changed, 33 insertions(+), 91 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 8038977..5e2f38f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -186,10 +186,6 @@ async function cmdInstall(force: boolean, nonInteractive: boolean): Promise` echoes the value to -// stdout on success. `config show` masks the same value to first-4-chars + -// ellipsis, but `set` had no equivalent masking — the literal key leaked into -// terminal scrollback, CI logs, and tool transcripts whenever the install -// flow ran. These tests pin the masking contract for `set` and guard the -// existing non-masking behavior of non-sensitive keys. +// `config set` must mask wandb_api_key in stdout (it didn't — see #66) but +// must still echo non-sensitive keys in full and persist the full secret. -import { test, suite, beforeEach, afterEach } from 'node:test'; +import { test } from 'node:test'; import assert from 'node:assert/strict'; import { spawn } from 'node:child_process'; import * as fs from 'node:fs'; @@ -19,101 +15,51 @@ import { fileURLToPath } from 'node:url'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = path.resolve(HERE, '..'); const CLI = path.join(REPO_ROOT, 'src', 'cli.ts'); - -// A distinctive secret so a substring match on stdout is unambiguous. const SECRET = 'wandb_v1_SUPERSECRETvalueDoNotLeak0123456789'; -interface Workspace { - home: string; - settingsFile: string; -} - -function newWorkspace(label: string): Workspace { +function newHome(label: string): { home: string; settingsFile: string } { const home = fs.mkdtempSync(`/tmp/wcp-cfgset-${label}-`); - const configDir = path.join(home, '.weave-claude-code'); - fs.mkdirSync(path.join(configDir, 'logs'), { recursive: true }); - const settingsFile = path.join(configDir, 'settings.json'); - fs.writeFileSync( - settingsFile, - JSON.stringify({ - log_file: path.join(configDir, 'logs', 'daemon.log'), - daemon_socket: path.join(configDir, 'daemon.sock'), - weave_project: null, - wandb_api_key: null, - debug: false, - installed_at: '2026-01-01T00:00:00Z', - version: '0.0.0-test', - }), - ); + const dir = path.join(home, '.weave-claude-code'); + fs.mkdirSync(path.join(dir, 'logs'), { recursive: true }); + const settingsFile = path.join(dir, 'settings.json'); + fs.writeFileSync(settingsFile, JSON.stringify({ + log_file: path.join(dir, 'logs', 'daemon.log'), + daemon_socket: path.join(dir, 'daemon.sock'), + weave_project: null, + wandb_api_key: null, + debug: false, + installed_at: '2026-01-01T00:00:00Z', + version: '0.0.0-test', + })); return { home, settingsFile }; } -function runCli(home: string, args: string[]): Promise<{ stdout: string; stderr: string; code: number | null }> { +function runCli(home: string, args: string[]): Promise<{ stdout: string; code: number | null }> { return new Promise((resolve, reject) => { - // Scrub WANDB_API_KEY/WEAVE_PROJECT from the parent env so they can't - // shadow what the CLI reads back from the per-test settings.json. const env = { ...process.env, HOME: home }; delete env.WANDB_API_KEY; delete env.WEAVE_PROJECT; - const child = spawn( - process.execPath, - ['--import', 'tsx', CLI, ...args], - { cwd: REPO_ROOT, env }, - ); + const child = spawn(process.execPath, ['--import', 'tsx', CLI, ...args], { cwd: REPO_ROOT, env }); let stdout = ''; - let stderr = ''; child.stdout.on('data', (b) => { stdout += b.toString(); }); - child.stderr.on('data', (b) => { stderr += b.toString(); }); child.on('error', reject); - child.on('exit', (code) => resolve({ stdout, stderr, code })); + child.on('exit', (code) => resolve({ stdout, code })); }); } -let workspaces: string[] = []; -beforeEach(() => { workspaces = []; }); -afterEach(() => { - for (const w of workspaces) fs.rmSync(w, { recursive: true, force: true }); -}); +test('config set: masks wandb_api_key, echoes weave_project in full', async () => { + const { home, settingsFile } = newHome('mask'); + try { + const apiKey = await runCli(home, ['config', 'set', 'wandb_api_key', SECRET]); + assert.equal(apiKey.code, 0); + assert.equal(apiKey.stdout.includes(SECRET), false, `stdout leaked the secret:\n${apiKey.stdout}`); + assert.match(apiKey.stdout, /wand…/); + assert.equal(JSON.parse(fs.readFileSync(settingsFile, 'utf8')).wandb_api_key, SECRET); -function workspace(label: string): Workspace { - const w = newWorkspace(label); - workspaces.push(w.home); - return w; -} - -suite('config set masks sensitive values in stdout', () => { - test('wandb_api_key: stdout does not contain the full secret', async () => { - const w = workspace('mask-stdout'); - const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); - assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); - assert.equal( - r.stdout.includes(SECRET), - false, - `stdout leaked the full secret:\n${r.stdout}`, - ); - }); - - test('wandb_api_key: stdout shows the masked prefix (first 4 chars + ellipsis)', async () => { - const w = workspace('mask-prefix'); - const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); - assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); - // Use the same shape `config show` already prints: `wand…` - assert.match(r.stdout, /wand…/, `stdout should contain masked prefix; got:\n${r.stdout}`); - }); - - test('wandb_api_key: full secret is still persisted to settings.json', async () => { - const w = workspace('mask-persist'); - const r = await runCli(w.home, ['config', 'set', 'wandb_api_key', SECRET]); - assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); - const saved = JSON.parse(fs.readFileSync(w.settingsFile, 'utf8')); - assert.equal(saved.wandb_api_key, SECRET, 'on-disk value must be the full secret'); - }); - - test('weave_project: non-sensitive value is still echoed in full', async () => { - const w = workspace('echo-project'); - const project = 'my-entity/my-project'; - const r = await runCli(w.home, ['config', 'set', 'weave_project', project]); - assert.equal(r.code, 0, `set should succeed — stderr=${r.stderr}`); - assert.match(r.stdout, new RegExp(project.replace('/', '\\/')), `stdout should echo full project; got:\n${r.stdout}`); - }); + const project = await runCli(home, ['config', 'set', 'weave_project', 'my-entity/my-project']); + assert.equal(project.code, 0); + assert.match(project.stdout, /my-entity\/my-project/); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } }); From ddd0108e6dcf43dcb76592b86b1478ce766f135d Mon Sep 17 00:00:00 2001 From: Rick Gao Date: Fri, 29 May 2026 09:36:06 -0700 Subject: [PATCH 3/3] refactor(cli): route cmdInstall mask calls through maskSecret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #66 — two remaining `value.slice(0, 4)}…` sites in cmdInstall (env-var notice and post-prompt echo) now go through the same helper as config show/set and status. Audited the rest of the codebase: no other call sites log secrets. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cli.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 5e2f38f..69593de 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -136,7 +136,7 @@ async function cmdInstall(force: boolean, nonInteractive: boolean): Promise'); } @@ -164,7 +164,7 @@ async function cmdInstall(force: boolean, nonInteractive: boolean): Promise)'); }