From 00fd6f2263eee5e5ad9136fb85a8aca0356b5595 Mon Sep 17 00:00:00 2001
From: oldschoola <f0rm4tm3@gmail.com>
Date: Tue, 23 Jun 2026 15:01:07 -0700
Subject: [PATCH] fix: handle colon-separator bypass and /join secrets in
 history filter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address three P1 code review comments on PR #3352:

1. Colon-separator bypass: parseSlashCommand() treats ':' as an argument
   separator, but shouldSkipHistory only split on whitespace. So
   /login:?code=abc&state=xyz bypassed the filter. Now uses the same
   earliest-whitespace-or-colon splitting as parseSlashCommand.

2. /join <link> secret: the collab join link carries a 32-byte room key
   and optional write token. Add /join to the denylist — skip any /join
   with arguments.

3. Added regression tests for colon-separator forms and /join denylist.
---
 .../src/modes/controllers/input-controller.ts | 28 +++++++++++++------
 .../slash-commands/history-security.test.ts   | 16 +++++++++++
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts
index 68ca0c3e4..744b04b79 100644
--- a/packages/coding-agent/src/modes/controllers/input-controller.ts
+++ b/packages/coding-agent/src/modes/controllers/input-controller.ts
@@ -29,20 +29,32 @@ import { generateSessionTitle, setSessionTerminalTitle } from "../../utils/title
 
 /**
  * Slash commands that may carry secrets in their arguments should never be
- * persisted to history. /login accepts three callback forms (redirect URL,
- * query string, raw auth code) — all can contain OAuth code=/state= params,
- * so skip any /login with arguments. /mcp add --token <token> carries a
- * bearer token.
+ * persisted to history.
+ *
+ * - /login accepts three callback forms (redirect URL, query string, raw auth
+ *   code) — all can contain OAuth code=/state= params.
+ * - /join <link> carries a 32-byte room key and optional write token.
+ * - /mcp add --token <token> carries a bearer token.
+ *
+ * The command name is extracted the same way as parseSlashCommand() — splitting
+ * on the earliest whitespace or colon — so /login:?code=... is correctly matched.
  */
 export function shouldSkipHistory(slashText: string): boolean {
 	if (!slashText.startsWith("/")) return false;
-	const name = slashText.slice(1).split(/\s+/, 1)[0];
+	const body = slashText.slice(1);
+	// Match parseSlashCommand: split on earliest whitespace or colon.
+	const firstWs = body.search(/\s/);
+	const firstColon = body.indexOf(":");
+	const sep = firstWs === -1 ? firstColon : firstColon === -1 ? firstWs : Math.min(firstWs, firstColon);
+	const name = sep === -1 ? body : body.slice(0, sep);
+	const hasArgs = sep !== -1;
 	// /login <anything> — parseCallbackInput() accepts redirect URLs, query
 	// strings (?code=...), and raw auth codes, all of which carry secrets.
-	// Skipping all /login-with-args is safer than pattern-matching each form.
-	if (name === "login" && slashText.length > "/login".length) return true;
+	if (name === "login" && hasArgs) return true;
+	// /join <link> — the link carries the 32-byte room key and write token.
+	if (name === "join" && hasArgs) return true;
 	if (name === "mcp") {
-		const args = slashText.slice("/mcp".length).trim();
+		const args = body.slice(sep + 1).trim();
 		return args.startsWith("add") && /--token\s/.test(args);
 	}
 	return false;
diff --git a/packages/coding-agent/test/slash-commands/history-security.test.ts b/packages/coding-agent/test/slash-commands/history-security.test.ts
index 6f327302e..0529670ec 100644
--- a/packages/coding-agent/test/slash-commands/history-security.test.ts
+++ b/packages/coding-agent/test/slash-commands/history-security.test.ts
@@ -18,6 +18,22 @@ describe("shouldSkipHistory — security filter for slash command history", () =
 		expect(shouldSkipHistory("/login raw-auth-code-xyz")).toBe(true);
 	});
 
+	it("skips /login when colon-separated (parseSlashCommand treats : as separator)", () => {
+		// /login:?code=abc&state=xyz — the colon is a valid separator, so the
+		// command name is "login" and the args carry the OAuth secret.
+		expect(shouldSkipHistory("/login:?code=abc&state=xyz")).toBe(true);
+		expect(shouldSkipHistory("/login:auth-code-xyz")).toBe(true);
+	});
+
+	it("skips /join with a link argument (carries 32-byte room key and write token)", () => {
+		expect(shouldSkipHistory("/join omp://share/abc123def456...")).toBe(true);
+		expect(shouldSkipHistory("/join omp:abc123def456...")).toBe(true);
+	});
+
+	it("does not skip /join without arguments", () => {
+		expect(shouldSkipHistory("/join")).toBe(false);
+	});
+
 	it("skips /mcp add with --token flag (contains bearer token)", () => {
 		expect(shouldSkipHistory("/mcp add myserver --url http://x --token sk-secret123")).toBe(true);
 	});