fix(network): keep short values and pseudo-headers out of secret profiles

README.md

@@ -351,10 +351,12 @@ may try to steer it, and that other programs on your computer may try to use the
 - **Less on disk.** Session journals are private (0700/0600), redact typed passwords, codes and tokens, and are pruned
   after 14 days (200 at most). HARs replace credentials with references; typed passwords and one-time codes are never
   stored. Secret profiles keep values out of transcripts and artifacts, but anything that runs with them (like
-  `secrets_run`) can use them. `secrets_run` commands get the relay's allowlisted environment plus the profile's
-  `BC_SECRET_N` values, not the environment you started opencode with: `PATH`, `HOME`, locale, `HTTP_PROXY`,
-  `HTTPS_PROXY`, `NO_PROXY`, `ALL_PROXY`, `SSL_CERT_FILE`, `SSL_CERT_DIR` and `NODE_EXTRA_CA_CERTS` pass through; other
-  variables (API keys included) don't.
+  `secrets_run`) can use them. Short captured values are redacted in place, while HTTP/2 pseudo-header values are
+  left as they are in HARs. Old profile slots holding either kind are dropped when loaded, because replacing them
+  everywhere corrupts unrelated text. `secrets_run` commands get the relay's allowlisted environment plus the
+  profile's `BC_SECRET_N` values, not the environment you started opencode with: `PATH`, `HOME`, locale, `HTTP_PROXY`,
+  `HTTPS_PROXY`, `NO_PROXY`, `ALL_PROXY`, `SSL_CERT_FILE`, `SSL_CERT_DIR` and `NODE_EXTRA_CA_CERTS` pass through;
+  other variables (API keys included) don't.
 
 Escape hatches. Set them in the environment you start opencode (or your MCP client) from; the relay inherits
 `OPENCODE_BROWSER_*` variables. Run `node cli/dist/cli.mjs relay restart` after changing them:

cli/src/auth/profile.ts

@@ -5,7 +5,7 @@ import fs from "node:fs/promises"
 import path from "node:path"
 import { terminateChildProcess } from "../update/child-process.ts"
 import { isNodeError, writeJsonFileAtomically } from "../fs-durability.ts"
-import { redactKnownValues, type CredentialSlot } from "../network-capture/redaction.ts"
+import { cleanCredentialSlots, highestSecretRef, redactOutputText, type CredentialSlot } from "../network-capture/redaction.ts"
 
 const StoredCredentialSlot = Schema.Struct({
   ref: Schema.String.check(Schema.isPattern(/^BC_SECRET_[1-9][0-9]*$/)),
@@ -20,6 +20,8 @@ const StoredAuthProfile = Schema.Struct({
   createdAt: Schema.String,
   updatedAt: Schema.String,
   slots: Schema.Array(StoredCredentialSlot),
+  // High-water mark for BC_SECRET_N refs. Optional so profiles written by older builds still read.
+  nextRef: Schema.optionalKey(Schema.Int.check(Schema.isGreaterThanOrEqualTo(1))),
 }).check(Schema.makeFilter((profile) => {
   const refs = profile.slots.map((slot) => slot.ref)
   if (new Set(refs).size !== refs.length) return "Auth profile refs must be unique"
@@ -29,6 +31,11 @@ const StoredAuthProfile = Schema.Struct({
 
 interface AuthProfile extends Schema.Schema.Type<typeof StoredAuthProfile> {}
 
+/** Never hand out a ref this profile has used, even when the slot that held it was dropped. */
+function nextSecretRef(profile: Pick<AuthProfile, "nextRef" | "slots">): number {
+  return Math.max(profile.nextRef ?? 1, highestSecretRef(profile.slots) + 1)
+}
+
 export type AuthProfileSummary = {
   readonly name: string
   readonly createdAt: string
@@ -82,7 +89,7 @@ const staleProfileLockMs = 60_000
 const maximumRunOutputBytes = 10_000_000
 const maximumTimeoutMs = 2_147_483_647
 
-export const read = Effect.fn("AuthProfile.read")(function* (name: string, options: { readonly baseDir?: string } = {}) {
+const readProfileFile = Effect.fnUntraced(function* (name: string, options: { readonly baseDir?: string } = {}) {
   const filePath = yield* profilePath(options.baseDir ?? defaultBaseDir(), name)
   const text = yield* Effect.tryPromise({
     try: () => fs.readFile(filePath, "utf8"),
@@ -100,12 +107,23 @@ export const read = Effect.fn("AuthProfile.read")(function* (name: string, optio
   )
 })
 
+export const read = Effect.fn("AuthProfile.read")(function* (name: string, options: { readonly baseDir?: string } = {}) {
+  const profile = yield* readProfileFile(name, options)
+  return { ...profile, slots: cleanCredentialSlots(profile.slots), nextRef: nextSecretRef(profile) }
+})
+
 export const readOptional = Effect.fn("AuthProfile.readOptional")(function* (name: string, options: { readonly baseDir?: string } = {}) {
   return yield* read(name, options).pipe(
     Effect.catchIf((error) => error.reason === "not-found", () => Effect.succeed(undefined)),
   )
 })
 
+const readRawOptional = Effect.fnUntraced(function* (name: string, options: { readonly baseDir?: string } = {}) {
+  return yield* readProfileFile(name, options).pipe(
+    Effect.catchIf((error) => error.reason === "not-found", () => Effect.succeed(undefined)),
+  )
+})
+
 export function withLock<A, E, R>(
   name: string,
   options: { readonly baseDir?: string },
@@ -161,28 +179,37 @@ export function withLock<A, E, R>(
 const writeProfile = Effect.fnUntraced(function* (options: {
   readonly name: string
   readonly slots: readonly CredentialSlot[]
+  readonly nextRef?: number
   readonly baseDir?: string
 }) {
   const baseDir = options.baseDir ?? defaultBaseDir()
   const filePath = yield* profilePath(baseDir, options.name)
-  const existing = yield* readOptional(options.name, { baseDir })
+  // Compare against the file as stored, not the cleaned view, so a profile polluted by an older
+  // build is rewritten even when its remaining slots did not change.
+  const existing = yield* readRawOptional(options.name, { baseDir })
   const now = new Date().toISOString()
+  const slots = cleanCredentialSlots(options.slots).map((slot) => ({
+    ref: slot.ref,
+    value: slot.value,
+    sources: [...slot.sources],
+    ...(slot.expiresAt ? { expiresAt: slot.expiresAt } : {}),
+  }))
   const profile: AuthProfile = {
     version: 1,
     name: options.name,
     createdAt: existing?.createdAt ?? now,
     updatedAt: now,
-    slots: options.slots.map((slot) => ({
-      ref: slot.ref,
-      value: slot.value,
-      sources: [...slot.sources],
-      ...(slot.expiresAt ? { expiresAt: slot.expiresAt } : {}),
-    })),
+    nextRef: Math.max(
+      existing ? nextSecretRef(existing) : 1,
+      options.nextRef ?? 1,
+      highestSecretRef(options.slots) + 1,
+    ),
+    slots,
   }
   yield* Schema.decodeUnknownEffect(StoredAuthProfile)(profile).pipe(
     Effect.mapError((cause) => new AuthProfileError({ message: `Auth profile has an invalid shape: ${options.name}`, operation: "write", reason: "invalid-profile", cause })),
   )
-  if (existing && sameSlots(existing.slots, profile.slots)) return summary(existing)
+  if (existing && existing.nextRef === profile.nextRef && sameSlots(existing.slots, profile.slots)) return summary(existing)
   yield* Effect.tryPromise({
     try: () => writeJsonFileAtomically(filePath, profile, { dirMode: 0o700 }),
     catch: (cause) => new AuthProfileError({ message: `Could not write auth profile: ${options.name}`, operation: "write", reason: "write-failed", cause }),
@@ -193,6 +220,7 @@ const writeProfile = Effect.fnUntraced(function* (options: {
 export const write = Effect.fn("AuthProfile.write")(function* (options: {
   readonly name: string
   readonly slots: readonly CredentialSlot[]
+  readonly nextRef?: number
   readonly baseDir?: string
 }) {
   return yield* writeLock.withPermit(writeProfile(options))
@@ -227,8 +255,8 @@ export const run = Effect.fn("AuthProfile.run")(function* (options: AuthRunOptio
     }).pipe(
       Effect.mapError((cause) => new AuthProfileError({ message: `Could not run command with auth profile ${options.name}`, operation: "run", reason: "run-failed", cause })),
     )
-  const stdout = redactOutput(result.stdout, profile.slots, maxOutputBytes)
-  const stderr = redactOutput(result.stderr, profile.slots, maxOutputBytes)
+  const stdout = redactRunOutput(result.stdout, profile.slots, maxOutputBytes)
+  const stderr = redactRunOutput(result.stderr, profile.slots, maxOutputBytes)
   return {
     ...result,
     stdout: stdout.text,
@@ -267,14 +295,18 @@ function sameSlots(left: readonly CredentialSlot[], right: readonly CredentialSl
   return JSON.stringify(left) === JSON.stringify(right)
 }
 
-function redactOutput(text: string, slots: readonly CredentialSlot[], maxBytes: number): { readonly text: string; readonly truncated: boolean } {
+/**
+ * Redact run output. Slots are cleaned first, so every remaining value is at least 8 characters and
+ * is replaced wherever it appears; an echoed value followed by a newline matches that substring.
+ */
+export function redactRunOutput(text: string, slots: readonly CredentialSlot[], maxBytes: number): { readonly text: string; readonly truncated: boolean } {
+  const usable = cleanCredentialSlots(slots)
   const bytes = Buffer.from(text)
   let cutoff = Math.min(bytes.length, maxBytes)
   let changed = true
   while (changed) {
     changed = false
-    for (const slot of slots) {
-      if (!slot.value) continue
+    for (const slot of usable) {
       const secret = Buffer.from(slot.value)
       let start = bytes.indexOf(secret)
       while (start >= 0 && start < cutoff) {
@@ -288,11 +320,13 @@ function redactOutput(text: string, slots: readonly CredentialSlot[], maxBytes:
     }
   }
   return {
-    text: redactKnownValues(bytes.subarray(0, cutoff).toString("utf8"), slots),
+    text: redactOutputText(bytes.subarray(0, cutoff).toString("utf8"), usable, noAlwaysRedacted),
     truncated: cutoff < bytes.length,
   }
 }
 
+const noAlwaysRedacted: ReadonlySet<string> = new Set()
+
 function runChild(options: {
   readonly command: string
   readonly args: readonly string[]

cli/src/network-capture/har-redaction.ts

@@ -30,7 +30,7 @@ export function finishCapture(
     const existingProfile = secrets
       ? yield* AuthProfile.readOptional(secrets, profileOptions)
       : undefined
-    const collector = new SecretCollector(existingProfile?.slots ?? [])
+    const collector = new SecretCollector(existingProfile?.slots ?? [], existingProfile?.nextRef ?? 1)
     for (const value of active.extraRedactions) collector.addRedactionOnly(value)
     const structurallyProtected = active.entries.map((entry) => protectEntry(entry, collector))
     const protectedEntries = structurallyProtected
@@ -44,7 +44,7 @@ export function finishCapture(
       })
     }
     const authProfile = secrets
-      ? yield* AuthProfile.write({ name: secrets, slots: collector.slots(), ...profileOptions })
+      ? yield* AuthProfile.write({ name: secrets, slots: collector.slots(), nextRef: collector.nextSecretRef(), ...profileOptions })
       : undefined
     if (options.outputPath) {
       yield* writeArtifact(options.outputPath, {

cli/src/network-capture/redaction.ts

@@ -20,6 +20,42 @@ const referencePattern = String.raw`BC_(?:SECRET|REDACTED)_\d+`
  * equals the known value is still replaced.
  */
 export const minimumKnownSubstringLength = 8
+// HTTP/2 pseudo-header sources are `METHOD url.request.header.:name` or
+// `METHOD url.response.header.:name`, optionally followed by a duplicate-header occurrence index.
+// Older profiles can have whitespace before the colon or after the name. A source grows as
+// `<scope>.` followed by query.… or fragment.…, or by (request|response).(header|json|form|multipart).…
+// So only query, fragment, request and response can directly follow the scope; json, form, multipart
+// and header only appear after request/response. Ambiguous URL paths ending in .query or .fragment
+// are kept: wrongly dropping a real secret is worse than keeping a hostname. :path carries the
+// request URL and its query slots, so it is usable.
+const pseudoHeaderSourcePattern = /^(?:[A-Z]+ (\S+)\.)?(?:request|response)\.header\.[ \t]*:[ \t]*([^.\s]+)[ \t]*(?:\.\d+)?$/
+const nonHeaderSourceSegmentPattern = /\.(?:query|fragment)(?:\.|$)|\.(?:request|response)\.(?:json|form|multipart|header)(?:\.|$)/
+function isPseudoHeaderSource(source: string): boolean {
+  const match = pseudoHeaderSourcePattern.exec(source)
+  if (!match) return false
+  const scope = match[1]
+  return (scope === undefined || !nonHeaderSourceSegmentPattern.test(scope)) && match[2] !== "path"
+}
+function isPseudoHeaderOnly(sources: readonly string[]): boolean {
+  return sources.length > 0 && sources.every(isPseudoHeaderSource)
+}
+/**
+ * Drop slots that cannot be used safely: values too short to replace without
+ * corrupting unrelated text, and slots that only ever came from an HTTP/2
+ * pseudo-header (an old :authority hostname, for example). Profiles written by
+ * older builds are cleaned through this before they are read or written back.
+ */
+export function cleanCredentialSlots(slots: readonly CredentialSlot[]): readonly CredentialSlot[] {
+  return slots.filter((slot) =>
+    slot.value.length >= minimumKnownSubstringLength && !isPseudoHeaderOnly(slot.sources))
+}
+/** The highest BC_SECRET_N number present, or 0. */
+export function highestSecretRef(slots: readonly CredentialSlot[]): number {
+  return slots.reduce((highest, slot) => {
+    const number = /^BC_SECRET_(\d+)$/.exec(slot.ref)?.[1]
+    return number === undefined ? highest : Math.max(highest, Number(number))
+  }, 0)
+}
 const secretHeaders = new Set([
   "authorization",
   "proxy-authorization",
@@ -29,14 +65,9 @@ const secretHeaders = new Set([
   "csrf-token",
   "x-xsrf-token",
 ])
-// HTTP/2 pseudo-headers (except :path, which is protected like a request URL) and standard CORS
-// response headers carry routing and policy, not credentials, and must never become secret slots.
+// Standard CORS response headers carry routing and policy, not credentials, and must never become
+// secret slots. HTTP/2 pseudo-headers are skipped structurally in protectHeader.
 const nonSecretHeaders = new Set([
-  ":authority",
-  ":method",
-  ":scheme",
-  ":status",
-  ":protocol",
   "access-control-allow-credentials",
   "access-control-allow-origin",
   "access-control-allow-methods",
@@ -65,8 +96,10 @@ export class SecretCollector {
   private updated = new Set<string>()
   private observed = new Set<string>()
 
-  constructor(existing: readonly CredentialSlot[] = []) {
-    for (const slot of existing) {
+  constructor(existing: readonly CredentialSlot[] = [], nextRef = 1) {
+    // Include refs whose slots are dropped: old HARs and scripts can still refer to them.
+    this.nextRef = Math.max(this.nextRef, nextRef, highestSecretRef(existing) + 1)
+    for (const slot of cleanCredentialSlots(existing)) {
       const copy: MutableCredentialSlot = {
         ref: slot.ref,
         value: slot.value,
@@ -87,11 +120,11 @@ export class SecretCollector {
   protectHeaders(headers: readonly { readonly name: string; readonly value: string }[], location: "request" | "response", requestScope = ""): Array<{ name: string; value: string }> {
     const occurrences = new Map<string, number>()
     return headers.map(({ name, value }) => {
-      const lowerName = name.toLowerCase()
+      const lowerName = name.trim().toLowerCase()
       const occurrence = occurrences.get(lowerName) ?? 0
       occurrences.set(lowerName, occurrence + 1)
       return {
-      name,
+        name,
         value: this.protectHeader(name, value, sourceName(requestScope, `${location}.header.${lowerName}`, occurrence), requestScope),
       }
     })
@@ -148,6 +181,11 @@ export class SecretCollector {
       .map((slot) => ({ ...slot, sources: [...slot.sources] }))
   }
 
+  /** The next reusable ref, including refs this collector dropped but the profile has used before. */
+  nextSecretRef(): number {
+    return this.nextRef
+  }
+
   updatedRefs(): readonly string[] {
     return Array.from(this.updated).sort()
   }
@@ -232,9 +270,12 @@ export class SecretCollector {
 
   private protectHeader(name: string, value: string, source: string, requestScope = ""): string {
     if (!value) return value
-    const lower = name.toLowerCase()
+    const lower = name.trim().toLowerCase()
     // :path carries the request path and query, so protect it exactly like the request URL.
     if (lower === ":path") return this.protectUrl(value, requestScope)
+    // Every other HTTP/2 pseudo-header is routing or status, never a credential. Checking the prefix
+    // matters because secretNamePattern would classify ":authority" as an auth header.
+    if (lower.startsWith(":")) return value
     if (nonSecretHeaders.has(lower)) return value
     if (lower === "cookie" || lower === "set-cookie") {
       const cookieOccurrences = new Map<string, number>()
@@ -324,6 +365,9 @@ export class SecretCollector {
       this.alwaysRedacted.add(value)
       return this.redactionReference(value)
     }
+    // Short values are redacted where they were captured but never stored as reusable slots:
+    // globally replacing "B" or "1" would corrupt unrelated output, including BC_SECRET_n refs.
+    if (value.length < minimumKnownSubstringLength) return this.redactionReference(value)
     const sourceRef = this.refsBySource.get(source)
     if (sourceRef) {
       const slot = this.slotsByRef.get(sourceRef)
@@ -494,13 +538,13 @@ export function redactKnownValues(text: string, slots: readonly RedactionSlot[])
 }
 
 /**
- * Redact execute output. Explicitly registered values (typed site secrets,
+ * Redact output text. Explicitly registered values (typed site secrets,
  * one-time request fields) and known values of at least
  * `minimumKnownSubstringLength` are replaced anywhere. A whole string equal to
  * any known value is replaced even when the value is shorter: output that is
  * exactly a credential is intentional credential output.
  */
-function redactOutputText(
+export function redactOutputText(
   text: string,
   slots: readonly RedactionSlot[],
   alwaysRedacted: ReadonlySet<string>,