diff --git a/AGENTS.md b/AGENTS.md index b68f6055..dc13b8c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -405,6 +405,15 @@ installation, players, backend services, the site, or production deployment. provisioning material, customer content, or secret-bearing headers/bodies. ffmpeg diagnostics reach the user only through `summarizeFfmpegError`, which redacts and bounds the stderr tail; keep it on that path. +- **No part of a stored credential reaches stdout, including the lookup + segment.** `redactToken` is gone: a redacted credential is `sr_live_***` + with no lookup id, and `redactText` / `redactValue` emit exactly that. A + command that wants to report a credential reports presence through + `hasToken` or `describeTokenPresence` (`present` / `(none)`), which is what + the `doctor` `token` row and `account show`'s `token_present` do. Never + reintroduce a truncated `sr_live_` value in a check detail, a human line, an + envelope, a log, or a problem detail. `tokenLookupId` stays for internal + correlation that never reaches output. - Transcoded bytes live in a `mkdtemp` directory the caller removes. Every failure path after that directory exists must remove it. - Playlist pages the CLI emits may use only `image`, `video`, `iframe`, and diff --git a/README.md b/README.md index 81ccafaf..be6c2e7b 100644 --- a/README.md +++ b/README.md @@ -555,6 +555,12 @@ through the npm package — passes even when optional pieces are absent. Stdout is a success envelope either way: branch on `data.status` and the rows, not on `ok`. +The `token` row reports presence and nothing else: `present` when this +installation holds a credential, `(none)` when it does not. Every segment of a +credential is secret, including the lookup id ahead of the final underscore, so +no prefix, suffix, or redacted form of the stored value appears in `doctor` +output in either mode. `account show` reports the same fact as `token_present`. + `node`, `config_permissions`, `token`, `api_url`, `ffmpeg`, `ffprobe`, `encoder_libx264`, `health`, `ready`, `version`, and `capabilities` fail when they are not satisfied. These rows warn instead: diff --git a/src/cli.test.ts b/src/cli.test.ts index 5430500a..ba178a61 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -2407,6 +2407,63 @@ test("doctor fails a missing libx264 because it is the default video profile", a } }); +/** + * ENG-5821. Every segment of a live credential is secret, including the lookup + * id ahead of the final underscore, so doctor reports presence and no part of + * the stored value reaches stdout in either output mode. + */ +test("doctor reports a configured credential as presence only", async () => { + resetFfmpegToolchainCache(); + const configDir = await testTemp("doctor-token-"); + const fsLike = { mkdir, open, rename, rm, chmod, stat, homedir: () => configDir, env: { XDG_CONFIG_HOME: configDir } }; + await writeConfigAtomic( + path.join(configDir, "screenrig", "config.json"), + { api_url: "https://api.screenrig.ai", token: "sr_live_tokidAAAAAAAAAAAAAAAA_secretsecretsecretsecretsecr" }, + fsLike, + ); + const probe = fakeToolchainProbe({ + encoders: ["libx264", "libx265", "libwebp", "libwebp_anim"], + filters: ["scale", "zscale", "tonemap"], + cwebp: true, + }) as unknown as NonNullable; + try { + const json = await withRuntime(["--json", "doctor"], memoryBackend(), { fs: fsLike, runProcess: probe }); + const envelope = JSON.parse(json.stdout) as { + data: { checks: Array<{ name: string; status: string; detail: string }> }; + }; + const token = envelope.data.checks.find((check) => check.name === "token"); + assert.ok(token, json.stdout); + assert.equal(token.status, "pass"); + assert.equal(token.detail, "present"); + for (const pattern of [/sr_live_/, /tokidAAAA/, /secretsecret/]) { + assert.doesNotMatch(json.stdout, pattern); + } + + const human = await withRuntime(["doctor"], memoryBackend(), { fs: fsLike, runProcess: probe }); + assert.match(human.stdout, /^PASS token: present$/m); + for (const pattern of [/sr_live_/, /tokidAAAA/, /secretsecret/]) { + assert.doesNotMatch(human.stdout, pattern); + } + } finally { + await rm(configDir, { recursive: true, force: true }); + } +}); + +test("doctor fails the token check without a credential", async () => { + resetFfmpegToolchainCache(); + const { code, stdout, configDir } = await withRuntime(["--json", "doctor"], memoryBackend()); + assert.equal(code, ExitCode.Unexpected); + const envelope = JSON.parse(stdout) as { + data: { status: string; checks: Array<{ name: string; status: string; detail: string }> }; + }; + const token = envelope.data.checks.find((check) => check.name === "token"); + assert.ok(token, stdout); + assert.equal(token.status, "fail"); + assert.equal(token.detail, "(none)"); + assert.doesNotMatch(stdout, /sr_live_/); + await rm(configDir, { recursive: true, force: true }); +}); + test("control-plane payloads and mutation idempotency match the v0.2 architecture", async () => { const transport = memoryBackend(); const configDir = await testTemp("contract-"); @@ -2841,11 +2898,14 @@ test("account show reports integer credit_remaining and warns when remaining is assert.equal(json.code, 0, json.stdout); const envelope = JSON.parse(json.stdout) as { ok: boolean; - data: { credit_remaining: number }; + data: { credit_remaining: number; token_present: boolean }; warnings: Array<{ code: string; message: string }>; }; assert.equal(envelope.ok, true); assert.equal(envelope.data.credit_remaining, 0); + // ENG-5821: the credential is reported as presence, never as a token shape. + assert.equal(envelope.data.token_present, true); + assert.doesNotMatch(json.stdout, /sr_live_|tokidAAAA/); const warning = envelope.warnings.find((item) => item.code === "credits_low"); assert.ok(warning, json.stdout); assert.match(warning.message, /\b0\b/); @@ -2855,6 +2915,8 @@ test("account show reports integer credit_remaining and warns when remaining is const human = await withTokenConfig(transport, ["account", "show"]); assert.equal(human.code, 0, human.stdout); assert.match(human.stdout, /credit_remaining: 0/); + assert.match(human.stdout, /^token: present$/m); + assert.doesNotMatch(human.stdout, /sr_live_|tokidAAAA/); assert.match(human.stdout, /warning: Remaining prepaid credit is 0, below 1000 credits\./); assert.doesNotMatch(human.stdout, /kCr|stripe|x402|mcr|millicredit|\$/i); }); diff --git a/src/commands.ts b/src/commands.ts index 99e38b03..8ce914a9 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -45,7 +45,8 @@ import { ApiClient, requireToken } from "./client.js"; import { preserveLogSocket, resolveConfig, - describeToken, + describeTokenPresence, + hasToken, readConfigFile, withConfigLock, writeConfigAtomic, @@ -1540,7 +1541,9 @@ async function accountShow(args: ParsedArgs, runtime: CliRuntime, resolved: Awai const token = requireToken(resolved.token); const client = clientFor(runtime, args, resolved.apiUrl, token); const response = await client.call({ method: "GET", path: "/api/v1/account" }); - const envelope = jsonBody(response, client.requestId, { token_lookup: describeToken(token) }); + // Presence only. The lookup segment of a credential identifies the live + // token, so no part of the stored value is reported on stdout. + const envelope = jsonBody(response, client.requestId, { token_present: hasToken(token) }); const account = response.body as Account; if (headerValue(response.headers, CREDITS_REMAINING_HEADER) === undefined) { observeCreditsRemaining(runtime, parseCreditsInteger(account.credit_remaining)); @@ -1552,7 +1555,7 @@ async function accountShow(args: ParsedArgs, runtime: CliRuntime, resolved: Awai ["id", account.id], ["revision", account.revision !== undefined ? String(account.revision) : undefined], ["credit_remaining", account.credit_remaining !== undefined ? String(account.credit_remaining) : undefined], - ["token", describeToken(token)], + ["token", describeTokenPresence(token)], ["request_id", client.requestId], ]), }; @@ -3309,8 +3312,8 @@ async function doctor( } checks.push({ name: "token", - status: resolved.token ? "pass" : "fail", - detail: describeToken(resolved.token), + status: hasToken(resolved.token) ? "pass" : "fail", + detail: describeTokenPresence(resolved.token), }); checks.push({ name: "api_url", diff --git a/src/config.ts b/src/config.ts index 4fe2a63d..9b79a3bb 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,7 +2,6 @@ import { chmod, mkdir, open, rename, rm, stat } from "node:fs/promises"; import path from "node:path"; import type { OperationLogger } from "./log/types.js"; import { configError } from "./problems.js"; - import { redactToken, tokenLookupId } from "./redact.js"; export interface ScreenRigConfig { api_url: string; @@ -425,10 +424,16 @@ import { chmod, mkdir, open, rename, rm, stat } from "node:fs/promises"; }; } - export function describeToken(token: string | undefined): string { - if (!token) { - return "(none)"; - } - const id = tokenLookupId(token); - return id ? redactToken(token) : "sr_live_***"; + /** Whether this installation holds a credential at all. */ + export function hasToken(token: string | undefined): boolean { + return typeof token === "string" && token.length > 0; + } + + /** + * Presence of a credential, for stdout. Every part of a live token is + * secret, including the lookup segment, so no shape, prefix, or suffix of + * the stored value is reported here. + */ + export function describeTokenPresence(token: string | undefined): string { + return hasToken(token) ? "present" : "(none)"; } diff --git a/src/redact.test.ts b/src/redact.test.ts index 0419ab05..ff2c6d57 100644 --- a/src/redact.test.ts +++ b/src/redact.test.ts @@ -14,12 +14,20 @@ test("redacts credentials and email addresses from errors", () => { assert.deepEqual( redactValue({ token, nested: [`email owner@example.com`] }), { - token: "sr_live_identifier_***", + token: "sr_live_***", nested: ["email [redacted-email]"], }, ); }); +test("a redacted credential keeps no lookup segment of the live token", () => { + const token = "sr_live_identifier_secret"; + assert.equal(redactText(`token ${token}`), "token sr_live_***"); + assert.doesNotMatch(redactText(`token ${token}`), /identifier/); + assert.doesNotMatch(JSON.stringify(redactValue({ token })), /identifier/); + assert.doesNotMatch(JSON.stringify(redactValue({ note: `use ${token}` })), /identifier/); +}); + test("redacts temporary agent connection authority and envelope material", () => { const temporary = `sac_${"A".repeat(43)}`; assert.equal(redactText(`ScreenRig-Agent-Connect ${temporary}`), "ScreenRig-Agent-Connect ***"); diff --git a/src/redact.ts b/src/redact.ts index fcc2ce85..84b50c8c 100644 --- a/src/redact.ts +++ b/src/redact.ts @@ -15,18 +15,18 @@ const SENSITIVE_KEY_RE = const SENSITIVE_VALUE_RE = /(sr_live_|sac_|Bearer\s|ScreenRig-Agent-Connect\s|data:image\/|#(link|provision)=|[?&](X-Amz-Signature|X-Goog-Signature|signature)=)/i; +/** + * Lookup segment of a credential, for internal correlation only. A redacted + * credential never carries it: the segment identifies the live token, so + * printing it hands an observer a usable half of the secret. + */ export function tokenLookupId(token: string): string | undefined { const match = /^sr_live_([A-Za-z0-9_-]+)_/.exec(token); return match?.[1]; } -export function redactToken(token: string): string { - const id = tokenLookupId(token); - if (!id) { - return "sr_live_***"; - } - return `sr_live_${id}_***`; -} +/** What a live credential looks like once redacted: the shape, and nothing else. */ +export const REDACTED_TOKEN = "sr_live_***"; export function isSensitiveKey(key: string): boolean { return SENSITIVE_KEY_RE.test(key); @@ -38,7 +38,7 @@ export function isSensitiveValue(value: string): boolean { export function redactText(value: string): string { return value - .replace(TOKEN_RE, (token) => redactToken(token)) + .replace(TOKEN_RE, REDACTED_TOKEN) .replace(AGENT_CONNECTION_TOKEN_RE, "sac_***") .replace(AGENT_CONNECTION_AUTH_RE, "ScreenRig-Agent-Connect ***") .replace(BEARER_RE, "Bearer ***") @@ -48,7 +48,7 @@ export function redactText(value: string): string { function redactSensitive(nested: unknown): unknown { if (typeof nested === "string" && tokenLookupId(nested)) { - return redactToken(nested); + return REDACTED_TOKEN; } return "***"; }