Merge PR #7067: fix(secrets): exact-match edits failing on files with credential-shaped tokens (@iacore)

packages/ai/CHANGELOG.md

@@ -24,6 +24,7 @@
 ### Changed
 
 - Anthropic OAuth requests now reproduce Cowork's current `claude-desktop` request profile, including client/runtime metadata, beta selection, system and billing attestation, the 64K output cap, and stable HTTP/1.1 header ordering.
+- Exported `SENSITIVE_TOKEN_RE` from `providers/transform-messages` so hosts can route the same credential shapes through reversible obfuscation instead of the irreversible redaction fallback ([#6968](https://github.com/can1357/oh-my-pi/issues/6968)).
 
 ## [17.2.0] - 2026-07-30
 

packages/ai/src/providers/transform-messages.ts

@@ -295,7 +295,15 @@ function normalizeAnthropicTargetToolCallId<TApi extends Api>(
  * - Preserves tool call structure (unlike converting to text summaries)
  * - Injects synthetic "aborted" tool results
  */
-const SENSITIVE_TOKEN_RE =
+/**
+ * Credential-shaped token patterns scrubbed from outbound provider traffic when
+ * credential redaction is enabled. Exported so hosts can route the same shapes
+ * through reversible obfuscation (keyed placeholders restored before local tool
+ * execution) instead of the irreversible `[*_token_redacted]` rewrite below —
+ * an irreversible placeholder echoed back in edit-tool `old_text` can never
+ * match the real bytes on disk.
+ */
+export const SENSITIVE_TOKEN_RE =
 	/(?<![a-zA-Z0-9_*-])(gh[opusr]_[a-zA-Z0-9_*]{36,}|github_pat_[a-zA-Z0-9_*]{36,}|glpat-[a-zA-Z0-9_*-]{20,}|sk-proj-[a-zA-Z0-9_*-]{36,}|sk-ant-[a-zA-Z0-9_*-]{36,}|sk-[a-zA-Z0-9_*-]{48,})(?![a-zA-Z0-9_*-])/gi;
 
 function hasPlausibleCredentialEntropy(token: string): boolean {

packages/coding-agent/CHANGELOG.md

@@ -83,6 +83,9 @@
 ### Fixed
 
 - Fixed automatic terminal appearance changes clearing native scrollback and snapping readers away from their current scroll position; the active output grid now repaints non-destructively, while Ctrl+L remains the explicit full-history recolor.
+### Fixed
+
+- Fixed exact-match edits failing on files containing credential-shaped tokens when `secrets.enabled` (Hide Secrets) is on: unconfigured API keys (GitHub/GitLab/OpenAI-shaped) in tool results now get reversible keyed placeholders from the secret obfuscator — restored byte-exact in tool-call arguments before execution — instead of pi-ai's irreversible `[*_token_redacted]` rewrite, so the model's `old_text` matches the real file bytes while credentials still never reach the provider ([#6968](https://github.com/can1357/oh-my-pi/issues/6968)).
 
 ## [17.2.0] - 2026-07-30
 

packages/coding-agent/src/sdk.ts

@@ -119,11 +119,13 @@ import lateDiagnosticTemplate from "./prompts/tools/lsp-late-diagnostic.md" with
 import { AgentLifecycleManager } from "./registry/agent-lifecycle";
 import { type AgentRef, AgentRegistry, MAIN_AGENT_ID } from "./registry/agent-registry";
 import {
+	builtinCredentialSecretEntries,
 	collectEnvSecrets,
 	deobfuscateSessionContext,
 	deobfuscateToolArguments,
 	getExistingSecretPlaceholderKey,
 	getSecretPlaceholderKey,
+	getSecretPlaceholderKeySync,
 	loadSecrets,
 	obfuscateMessages,
 	obfuscateProviderContext,
@@ -1359,21 +1361,26 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 	if (settings.get("secrets.enabled")) {
 		const fileEntries = await logger.time("loadSecrets", loadSecrets, cwd, agentDir);
 		const envEntries = collectEnvSecrets();
-		const allEntries = [...envEntries, ...fileEntries];
-		const needsPlaceholderKey = secretEntriesNeedPlaceholderKey(allEntries);
+		// Built-in credential-pattern entries come last so user-configured entries
+		// (plain literals, custom regexes) take precedence in the scan order.
+		const allEntries = [...envEntries, ...fileEntries, ...builtinCredentialSecretEntries()];
+		// Only CONFIGURED entries force startup key creation: a configured
+		// obfuscate-mode secret — or a default (no custom `replacement`)
+		// replace-mode regex whose key-derived idempotent fallback marker needs a
+		// stable key across restarts (see `secretEntryNeedsPlaceholderKey`) —
+		// mints placeholders as soon as the obfuscator is built. The built-in
+		// credential-pattern entry matches dynamically, so it resolves the
+		// persisted key lazily on first match instead of creating the key file
+		// for every secrets.enabled session; a session whose content never
+		// contains a credential-shaped token must not require the key, otherwise a
+		// headless run on an unwritable default config root pays for a feature it
+		// does not use.
+		const needsPlaceholderKey = secretEntriesNeedPlaceholderKey([...envEntries, ...fileEntries]);
 		const placeholderKey = needsPlaceholderKey
 			? await getSecretPlaceholderKey(agentDir)
 			: await getExistingSecretPlaceholderKey(agentDir);
 		if (allEntries.length > 0) {
-			// The persisted placeholder key — and creating its key file under the
-			// configured agentDir — is only needed for reversible obfuscate-mode
-			// placeholders, or for a default (no custom `replacement`) replace-mode
-			// regex whose key-derived idempotent fallback marker needs a stable key
-			// across restarts (see `secretEntryNeedsPlaceholderKey`). A replace-only
-			// secrets set with no such regex must not require the key; otherwise a
-			// headless run with an unwritable default config root fails startup for a
-			// feature it does not use.
-			obfuscator = new SecretObfuscator(allEntries, placeholderKey);
+			obfuscator = new SecretObfuscator(allEntries, placeholderKey ?? (() => getSecretPlaceholderKeySync(agentDir)));
 		}
 		if (obfuscator?.hasSecrets() !== true && placeholderKey !== undefined) {
 			// No configured entry produced an active secret (e.g. only ignored short

packages/coding-agent/src/secrets/index.ts

@@ -1,6 +1,7 @@
 import * as crypto from "node:crypto";
-import * as fs from "node:fs/promises";
+import * as fs from "node:fs";
 import * as path from "node:path";
+import { SENSITIVE_TOKEN_RE } from "@oh-my-pi/pi-ai/providers/transform-messages";
 import { getAgentDir, isEnoent, logger } from "@oh-my-pi/pi-utils";
 import { YAML } from "bun";
 import { regexHasUnresolvableShortMatchFallback, type SecretEntry, sanitizeSecretFriendlyName } from "./obfuscator";
@@ -31,9 +32,9 @@ export async function getSecretPlaceholderKey(keyDir: string = getAgentDir()): P
 	}
 
 	const generated = crypto.randomBytes(32).toString("base64url");
-	await fs.mkdir(keyDir, { recursive: true });
+	await fs.promises.mkdir(keyDir, { recursive: true });
 	try {
-		await fs.writeFile(keyPath, generated, { flag: "wx", mode: 0o600 });
+		await fs.promises.writeFile(keyPath, generated, { flag: "wx", mode: 0o600 });
 		cachedPlaceholderKeys.set(keyPath, generated);
 		return generated;
 	} catch (err) {
@@ -71,6 +72,60 @@ export async function getExistingSecretPlaceholderKey(keyDir: string = getAgentD
 	return existing;
 }
 
+// Process-stable fallback for the sync lazy path when the key file cannot be
+// persisted (e.g. unwritable config root on a headless run). Memoized so
+// re-obfuscation within the process stays idempotent; placeholders simply lose
+// cross-session stability, matching `defaultPlaceholderKey()` in obfuscator.ts.
+let ephemeralSyncPlaceholderKey: string | undefined;
+
+/**
+ * Synchronous variant of `getSecretPlaceholderKey` for the lazy key provider
+ * `SecretObfuscator` invokes inside its synchronous `obfuscate()` path when a
+ * built-in credential-pattern entry first matches session content. Never
+ * throws: an unreadable or unwritable key file degrades to a process-ephemeral
+ * key (with a warning) instead of breaking the session.
+ */
+export function getSecretPlaceholderKeySync(keyDir: string = getAgentDir()): string {
+	const keyPath = path.join(keyDir, "secret-placeholder.key");
+	const cached = cachedPlaceholderKeys.get(keyPath);
+	if (cached !== undefined) return cached;
+	try {
+		const existing = fs.readFileSync(keyPath, "utf8").trim();
+		if (PLACEHOLDER_KEY_RE.test(existing)) {
+			cachedPlaceholderKeys.set(keyPath, existing);
+			return existing;
+		}
+	} catch {
+		// Missing or unreadable — attempt creation below.
+	}
+	const generated = crypto.randomBytes(32).toString("base64url");
+	try {
+		fs.mkdirSync(keyDir, { recursive: true });
+		fs.writeFileSync(keyPath, generated, { flag: "wx", mode: 0o600 });
+		cachedPlaceholderKeys.set(keyPath, generated);
+		return generated;
+	} catch (err) {
+		if ((err as NodeJS.ErrnoException).code === "EEXIST") {
+			// Another process won the create race; accept its key if valid.
+			try {
+				const winner = fs.readFileSync(keyPath, "utf8").trim();
+				if (PLACEHOLDER_KEY_RE.test(winner)) {
+					cachedPlaceholderKeys.set(keyPath, winner);
+					return winner;
+				}
+			} catch {
+				// Fall through to the ephemeral key.
+			}
+		}
+		logger.warn("Could not persist secret placeholder key, using a process-ephemeral key", {
+			path: keyPath,
+			error: String(err),
+		});
+		ephemeralSyncPlaceholderKey ??= crypto.randomBytes(32).toString("base64url");
+		return ephemeralSyncPlaceholderKey;
+	}
+}
+
 /** Read and validate the key file, optionally retrying briefly until a valid key lands. */
 async function readPlaceholderKeyFile(keyPath: string, retry: boolean): Promise<string | undefined> {
 	const attempts = retry ? 50 : 1;
@@ -145,6 +200,31 @@ export function collectEnvSecrets(): SecretEntry[] {
 	return entries;
 }
 
+/**
+ * Built-in entries covering credential-shaped tokens (GitHub/GitLab/OpenAI-style
+ * API keys) that are NOT configured via secrets.yml or the environment. Without
+ * these, such a token in a tool result falls through to pi-ai's irreversible
+ * provider-boundary redaction (`[openai_token_redacted]`); the model then echoes
+ * that placeholder into edit-tool `old_text`, which can never match the real
+ * bytes on disk (issue #6968). Routing the same shapes through the obfuscator
+ * mints reversible keyed placeholders that `deobfuscateToolArguments` restores
+ * before tool execution, keeping exact-match edits working while the credential
+ * bytes still never reach the provider. Unlike the pi-ai redaction there is no
+ * entropy gate here — a false positive only over-obfuscates, which stays
+ * transparent because the round trip is lossless.
+ */
+export function builtinCredentialSecretEntries(): SecretEntry[] {
+	return [
+		{
+			type: "regex",
+			content: SENSITIVE_TOKEN_RE.source,
+			flags: "i",
+			mode: "obfuscate",
+			friendlyName: "Credential",
+		},
+	];
+}
+
 async function loadSecretsFile(filePath: string): Promise<SecretEntry[]> {
 	try {
 		const text = await Bun.file(filePath).text();

packages/coding-agent/src/secrets/obfuscator.ts

@@ -594,18 +594,22 @@ export class SecretObfuscator {
 	/** Whether any secrets were configured */
 	#hasAny: boolean;
 
-	/** Private per-install (or per-process) key for the keyed placeholder digest. */
-	readonly #key: string;
+	/**
+	 * Private per-install (or per-process) key for the keyed placeholder digest.
+	 * Resolved lazily when the constructor received a key PROVIDER: the first
+	 * placeholder mint or keyed fallback marker triggers resolution, so callers
+	 * can defer persisting `secret-placeholder.key` until a dynamic regex entry
+	 * actually matches session content.
+	 */
+	#key: string | undefined;
+	#keyProvider: (() => string) | undefined;
 
-	constructor(entries: SecretEntry[], key: string = defaultPlaceholderKey()) {
-		this.#key = key;
-		// The keyed-hash key makes obfuscate-mode placeholder bases un-dictionaryable,
-		// but it can be persisted in a user-readable file (`secret-placeholder.key`).
-		// A prompt-injected tool read (read/bash) could otherwise surface it to the
-		// provider verbatim and undo that protection, so redact the key itself from
-		// obfuscated (provider-visible) output as a one-way secret.
-		this.#replaceMappings.set(key, this.#generateSecretReplacement(key));
-		this.#configuredSecretValues.add(key);
+	constructor(entries: SecretEntry[], key: string | (() => string) = defaultPlaceholderKey()) {
+		if (typeof key === "function") {
+			this.#keyProvider = key;
+		} else {
+			this.#setPlaceholderKey(key);
+		}
 		// Collect every configured plain-secret literal AND compile every regex
 		// entry BEFORE minting any placeholder below, so a placeholder's friendly
 		// name (checked against both in `#createPlaceholder`) can never embed a
@@ -671,6 +675,30 @@ export class SecretObfuscator {
 		this.#hasAny = hasRealSec;
 	}
 
+	/**
+	 * The keyed-hash key makes obfuscate-mode placeholder bases un-dictionaryable,
+	 * but it can be persisted in a user-readable file (`secret-placeholder.key`).
+	 * A prompt-injected tool read (read/bash) could otherwise surface it to the
+	 * provider verbatim and undo that protection, so redact the key itself from
+	 * obfuscated (provider-visible) output as a one-way secret.
+	 */
+	#setPlaceholderKey(key: string): void {
+		this.#key = key;
+		this.#replaceMappings.set(key, this.#generateSecretReplacement(key));
+		this.#configuredSecretValues.add(key);
+	}
+
+	/** Resolve the placeholder key once, minting its self-redaction on first use. */
+	#getKey(): string {
+		let key = this.#key;
+		if (key === undefined) {
+			key = this.#keyProvider?.() ?? defaultPlaceholderKey();
+			this.#keyProvider = undefined;
+			this.#setPlaceholderKey(key);
+		}
+		return key;
+	}
+
 	hasSecrets(): boolean {
 		return this.#hasAny;
 	}
@@ -937,7 +965,9 @@ export class SecretObfuscator {
 		// remainder bytes that merely look sentinel-shaped (e.g. `ZZZZ`) cannot equal
 		// the marker and are still redacted instead of passed through.
 		const replacement =
-			chunk.length <= 2 ? "Z".repeat(chunk.length) : `ZZ${buildKeyedReplacementRun(this.#key, chunk.length - 2)}`;
+			chunk.length <= 2
+				? "Z".repeat(chunk.length)
+				: `ZZ${buildKeyedReplacementRun(this.#getKey(), chunk.length - 2)}`;
 		this.#generatedReplaceChunks.add(replacement);
 		return replacement;
 	}
@@ -1023,7 +1053,9 @@ export class SecretObfuscator {
 			// the ordinary keyed-run fallback #generateReplacement already uses.
 			replacement =
 				stable ??
-				(value.length <= 2 ? buildKeyedReplacementRun(this.#key, value.length) : this.#generateReplacement(value));
+				(value.length <= 2
+					? buildKeyedReplacementRun(this.#getKey(), value.length)
+					: this.#generateReplacement(value));
 			regex.lastIndex = 0;
 		}
 		this.#generatedReplaceChunks.add(replacement);
@@ -1183,7 +1215,9 @@ export class SecretObfuscator {
 
 		for (let attempt = 0; ; attempt++) {
 			const base =
-				attempt === 0 ? buildHashBase(this.#key, baseKey) : buildHashBase(this.#key, `${baseKey}\0${attempt}`);
+				attempt === 0
+					? buildHashBase(this.#getKey(), baseKey)
+					: buildHashBase(this.#getKey(), `${baseKey}\0${attempt}`);
 			const owner = this.#placeholderBaseOwners.get(base);
 			if (owner !== undefined && owner !== baseKey) continue;
 			this.#placeholderBaseOwners.set(base, baseKey);
@@ -1195,7 +1229,7 @@ export class SecretObfuscator {
 	#reserveFallbackPlaceholderBase(baseKey: string, startAttempt: number): string {
 		for (let attempt = startAttempt; ; attempt++) {
 			const owner = `${baseKey}\0collision\0${attempt}`;
-			const base = buildHashBase(this.#key, `${baseKey}\0collision\0${attempt}`);
+			const base = buildHashBase(this.#getKey(), `${baseKey}\0collision\0${attempt}`);
 			if (this.#placeholderBaseOwners.has(base)) continue;
 			this.#placeholderBaseOwners.set(base, owner);
 			return base;

packages/coding-agent/test/secrets-obfuscator.test.ts

@@ -3,14 +3,17 @@
  */
 
 import { describe, expect, it, spyOn } from "bun:test";
+import * as crypto from "node:crypto";
 import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
 import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
 import type { AssistantMessage, Context, Message, TextContent } from "@oh-my-pi/pi-ai";
 import {
+	builtinCredentialSecretEntries,
 	getExistingSecretPlaceholderKey,
 	getSecretPlaceholderKey,
+	getSecretPlaceholderKeySync,
 	loadSecrets,
 } from "@oh-my-pi/pi-coding-agent/secrets";
 import {
@@ -51,6 +54,69 @@ describe("compileSecretRegex", () => {
 	});
 });
 
+describe("builtinCredentialSecretEntries", () => {
+	// Issue #6968: an unconfigured credential-shaped token in a tool result used
+	// to fall through to pi-ai's irreversible `[*_token_redacted]` rewrite, so an
+	// edit-tool `old_text` echoing that placeholder could never match the file.
+	// The contract: the token is hidden from provider-visible text AND restored
+	// byte-exact in tool-call arguments before tool execution.
+	it("hides unconfigured credential-shaped tokens and restores them in tool-call arguments", () => {
+		const obfuscator = new SecretObfuscator(builtinCredentialSecretEntries());
+		expect(obfuscator.hasSecrets()).toBe(true);
+
+		const tokens = [`sk-${"a1B-c2D".repeat(7)}e3F`, `ghp_${"aB1".repeat(12)}`, `glpat-${"xY2-".repeat(5)}`];
+		for (const token of tokens) {
+			const fileLine = `MOONSHOT_API_KEY=${token}`;
+			const providerView = obfuscator.obfuscate(fileLine);
+			expect(providerView).not.toContain(token);
+			// Re-obfuscation is a fixed point: the placeholder itself is never re-matched.
+			expect(obfuscator.obfuscate(providerView)).toBe(providerView);
+			// The model echoes the placeholder into edit-tool old_text verbatim.
+			const args = deobfuscateToolArguments(obfuscator, { old_text: providerView });
+			expect(args.old_text).toBe(fileLine);
+		}
+	});
+});
+
+describe("lazy placeholder key", () => {
+	// The built-in credential entries match dynamically, so they must not force
+	// `secret-placeholder.key` creation for every secrets.enabled session: the
+	// key provider is only invoked when a credential-shaped token is actually
+	// obfuscated, and exactly once across the obfuscator's lifetime.
+	it("resolves the key provider only on the first real credential match", () => {
+		let resolutions = 0;
+		const obfuscator = new SecretObfuscator(builtinCredentialSecretEntries(), () => {
+			resolutions++;
+			return crypto.randomBytes(32).toString("base64url");
+		});
+		expect(resolutions).toBe(0);
+		obfuscator.obfuscate("MOONSHOT_API_KEY=huntsville");
+		expect(resolutions).toBe(0);
+
+		const token = `sk-${"a1B-c2D".repeat(7)}e3F`;
+		const providerView = obfuscator.obfuscate(`MOONSHOT_API_KEY=${token}`);
+		expect(resolutions).toBe(1);
+		expect(providerView).not.toContain(token);
+		obfuscator.obfuscate(`repeat: ${token}`);
+		expect(resolutions).toBe(1);
+		const args = deobfuscateToolArguments(obfuscator, { old_text: providerView });
+		expect(args.old_text).toBe(`MOONSHOT_API_KEY=${token}`);
+	});
+
+	it("getSecretPlaceholderKeySync creates the key file on demand and shares it with the async readers", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-lazy-placeholder-key-"));
+		try {
+			expect(await getExistingSecretPlaceholderKey(dir)).toBeUndefined();
+			const key = getSecretPlaceholderKeySync(dir);
+			expect(key).toMatch(/^[A-Za-z0-9_-]{43}$/);
+			expect(await getExistingSecretPlaceholderKey(dir)).toBe(key);
+			expect(await getSecretPlaceholderKey(dir)).toBe(key);
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+});
+
 describe("SecretObfuscator regex behavior", () => {
 	it("obfuscates and deobfuscates regex matches with flags", () => {
 		const obfuscator = new SecretObfuscator([{ type: "regex", content: "api[_-]?key\\s*=\\s*\\w+", flags: "i" }]);