fix(workspace): redact GitHub tokens in container logs

src/orchestrator/orchestrator.ts

@@ -61,7 +61,7 @@ function buildAuthenticatedCloneUrl(
     // Backward-compatibility: older rows may still contain raw PAT strings.
     // Use the raw value as a last resort so private HTTPS clones keep working.
     token = encryptedToken;
-    log.warn({ integrationType, cloneUrl: rawCloneUrl, err }, "failed to decrypt token for authenticated clone URL; falling back to raw token value");
+    log.warn({ integrationType, cloneUrl: redactUrls(rawCloneUrl), err }, "failed to decrypt token for authenticated clone URL; falling back to raw token value");
   }
   if (/^\*{4,}$/.test(token)) return undefined;
   const usernamePrefix = integrationType === "github" ? "x-access-token" : "oauth2";

src/utils/redactUrl.ts

@@ -1,9 +1,45 @@
+/** Token-shaped values that must be masked regardless of the surrounding key. */
+const TOKEN_VALUE = /(gh[opusr]_[A-Za-z0-9]{16,})|(github_pat_[A-Za-z0-9_]{16,})/g;
+
 /**
- * Redact credentials embedded in HTTP(S) URLs.
+ * Redact credentials embedded in HTTP(S) URLs and GitHub token-shaped values.
  *
  * Replaces `https://user:secret@host/...` with `https://<redacted>@host/...`
  * so tokens never appear in logs, error messages, or external comments.
  */
 export function redactUrls(text: string): string {
-  return text.replace(/(https?:\/\/)[^@\s/][^@\s]*@/g, "$1<redacted>@");
+  return text
+    .replace(/(https?:\/\/)[^/\s?#@]+@/gi, "$1<redacted>@")
+    .replace(TOKEN_VALUE, "<redacted>");
+}
+
+/** Env-var names whose values must never be logged. */
+const SENSITIVE_KEY =
+  /(?:^|[_-])(?:TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?|PRIVATE[_-]?KEY|API[_-]?KEY|KEY|AUTH|PAT)(?:$|[_-])/i;
+
+function redactSensitiveAssignment(arg: string): string | undefined {
+  const prefix = arg.startsWith("--env=") ? "--env=" : "";
+  const assignment = prefix ? arg.slice(prefix.length) : arg;
+  const separator = assignment.indexOf("=");
+  if (separator <= 0 || !SENSITIVE_KEY.test(assignment.slice(0, separator))) {
+    return undefined;
+  }
+  return `${prefix}${assignment.slice(0, separator)}=<redacted>`;
+}
+
+/**
+ * Redact secrets from a Docker argv array before logging.
+ *
+ * Masks the value of any `NAME=VALUE` argument whose name looks sensitive
+ * (e.g. `GITHUB_TOKEN=...`, `COPILOT_SDK_AUTH_TOKEN=...`), strips credentials
+ * embedded in URLs, and masks any stray GitHub token-shaped values.
+ */
+export function redactDockerArgs(args: readonly string[]): string[] {
+  return args.map((arg) => {
+    const redactedAssignment = redactSensitiveAssignment(arg);
+    if (redactedAssignment !== undefined) {
+      return redactedAssignment;
+    }
+    return redactUrls(arg);
+  });
 }

src/vcs/githubVcsConnector.ts

@@ -49,7 +49,10 @@ export class GitHubVcsConnector implements VcsConnector {
 
   /** Clone a GitHub repository via HTTPS into the target directory. */
   async clone(repoUrl: string, branch: string, targetDir: string): Promise<void> {
-    log.info({ repoUrl, branch, targetDir }, "cloning repository from GitHub via HTTPS");
+    log.info(
+      { repoUrl: redactUrls(repoUrl), branch, targetDir },
+      "cloning repository from GitHub via HTTPS"
+    );
 
     try {
       execFileSync("git", ["clone", "--branch", branch, "--depth", "1", repoUrl, targetDir], {
@@ -60,7 +63,9 @@ export class GitHubVcsConnector implements VcsConnector {
       log.info({ targetDir }, "repository cloned successfully");
     } catch (err: unknown) {
       const error = err instanceof Error ? err : new Error(String(err));
-      throw new Error(`Failed to clone ${repoUrl}: ${error.message.slice(0, 500)}`);
+      throw new Error(
+        redactUrls(`Failed to clone ${repoUrl}: ${error.message.slice(0, 500)}`)
+      );
     }
   }
 

src/vcs/gitlabVcsConnector.ts

@@ -46,7 +46,7 @@ export class GitLabVcsConnector implements VcsConnector {
   /** Clone a GitLab repository via HTTP into the target directory. */
   async clone(repoUrl: string, branch: string, targetDir: string): Promise<void> {
     log.info(
-      { repoUrl, branch, targetDir },
+      { repoUrl: redactUrls(repoUrl), branch, targetDir },
       "cloning repository from GitLab via HTTP"
     );
 
@@ -61,7 +61,7 @@ export class GitLabVcsConnector implements VcsConnector {
     } catch (err: unknown) {
       const error = err instanceof Error ? err : new Error(String(err));
       throw new Error(
-        `Failed to clone GitLab repository: ${error.message.slice(0, 300)}`
+        redactUrls(`Failed to clone GitLab repository: ${error.message.slice(0, 300)}`)
       );
     }
   }

src/workspace/workspaceRunner.ts

@@ -14,6 +14,7 @@ import type {
 import type { AgentAdapter } from "../interfaces.js";
 import { createVolume, removeVolume, execInVolume, stopContainersUsingVolume } from "./dockerVolume.js";
 import { getLogger } from "../logger.js";
+import { redactUrls, redactDockerArgs } from "../utils/redactUrl.js";
 import { buildSkillsCliArgs, isSshSkillSource, parseRemoteSkillSources, skillsAgentId, sshSkillSourceCommandPort } from "./skillSources.js";
 import type { AgentProvider } from "./skillSources.js";
 
@@ -220,7 +221,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
 
     dockerArgs.push(spec.image, ...spec.command);
 
-    log.debug({ taskId: context.taskId, dockerArgs }, "agent container command");
+    log.debug({ taskId: context.taskId, dockerArgs: redactDockerArgs(dockerArgs) }, "agent container command");
 
     const stdoutChunks: string[] = [];
     const stderrChunks: string[] = [];
@@ -322,7 +323,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
   ): Promise<CloneResult> {
     try {
       log.info(
-        { taskId: handle.taskId, repoUrl, branch, volume: handle.volumeName },
+        { taskId: handle.taskId, repoUrl: redactUrls(repoUrl), branch, volume: handle.volumeName },
         "cloning repository into volume"
       );
       const result = await execInVolume({
@@ -337,9 +338,9 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
       }
       return { success: true, localPath: "/workspace" };
     } catch (err) {
-      const errorMsg = err instanceof Error ? err.message : String(err);
+      const errorMsg = redactUrls(err instanceof Error ? err.message : String(err));
       log.error(
-        { taskId: handle.taskId, repoUrl, branch, error: errorMsg },
+        { taskId: handle.taskId, repoUrl: redactUrls(repoUrl), branch, error: errorMsg },
         "failed to clone repository"
       );
       return { success: false, localPath: "/workspace", error: errorMsg };
@@ -392,7 +393,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
         taskId: handle.taskId,
         targetCount: sorted.length,
         rootRepoKey: root.repoKey,
-        rootCloneUrl: root.cloneUrl,
+        rootCloneUrl: redactUrls(root.cloneUrl),
       },
       "preparing project workspace (multi push target via volume)"
     );
@@ -407,7 +408,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
       ...(sshKnownHostsPath !== undefined ? { sshKnownHostsPath } : {}),
     });
     if (rootResult.exitCode !== 0) {
-      const errorMsg = rootResult.stderr.slice(0, 500);
+      const errorMsg = redactUrls(rootResult.stderr.slice(0, 500));
       log.error(
         { taskId: handle.taskId, repoKey: root.repoKey, error: errorMsg },
         "root push target clone failed"
@@ -441,7 +442,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
         ...(sshKnownHostsPath !== undefined ? { sshKnownHostsPath } : {}),
       });
       if (cloneResult.exitCode !== 0) {
-        const errorMsg = cloneResult.stderr.slice(0, 300);
+        const errorMsg = redactUrls(cloneResult.stderr.slice(0, 300));
         log.warn(
           { taskId: handle.taskId, repoKey: target.repoKey, localPath: target.localPath, error: errorMsg },
           "push target clone failed; continuing with remaining targets"
@@ -677,7 +678,7 @@ export class DockerWorkspaceRunner implements WorkspaceRunner {
 
     dockerArgs.push(spec.image, ...spec.command);
 
-    log.debug({ taskId: handle.taskId, dockerArgs }, "review container command");
+    log.debug({ taskId: handle.taskId, dockerArgs: redactDockerArgs(dockerArgs) }, "review container command");
 
     const stdoutChunks: string[] = [];
     const stderrChunks: string[] = [];

tests/unit/githubVcsConnector.test.ts

@@ -1,7 +1,23 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { execFileSync } from "child_process";
 import { GitHubVcsConnector } from "../../src/vcs/githubVcsConnector.js";
 import { jsonResponse, errorResponse } from "./helpers/fixtures.js";
 
+const { logger } = vi.hoisted(() => ({
+  logger: {
+    trace: vi.fn(),
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    fatal: vi.fn(),
+  },
+}));
+
+vi.mock("../../src/logger.js", () => ({
+  getLogger: vi.fn(() => logger),
+}));
+
 const API_BASE_URL = "https://api.github.com";
 const HOST = "github.com";
 const OWNER = "octocat";
@@ -77,6 +93,37 @@ describe("GitHubVcsConnector", () => {
     });
   });
 
+  describe("clone", () => {
+    it("redacts credentials from logs without changing the Git command", async () => {
+      const repoUrl =
+        "https://x-access-token:ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345@github.com/octocat/hello-world.git";
+
+      await makeConnector().clone(repoUrl, "main", "/tmp/repo");
+
+      expect(vi.mocked(execFileSync)).toHaveBeenCalledWith(
+        "git",
+        ["clone", "--branch", "main", "--depth", "1", repoUrl, "/tmp/repo"],
+        expect.any(Object)
+      );
+      expect(logger.info).toHaveBeenCalledWith(
+        expect.objectContaining({ repoUrl: "https://<redacted>@github.com/octocat/hello-world.git" }),
+        "cloning repository from GitHub via HTTPS"
+      );
+    });
+
+    it("redacts credentials from clone failures", async () => {
+      const repoUrl =
+        "https://x-access-token:ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345@github.com/octocat/hello-world.git";
+      vi.mocked(execFileSync).mockImplementationOnce(() => {
+        throw new Error(`fatal: unable to access '${repoUrl}'`);
+      });
+
+      await expect(makeConnector().clone(repoUrl, "main", "/tmp/repo")).rejects.not.toThrow(
+        /ghp_/
+      );
+    });
+  });
+
   describe("getChangeStatus", () => {
     it("returns OPEN for open PRs", async () => {
       fetchMock.mockResolvedValueOnce(jsonResponse(PR_RESPONSE));

tests/unit/gitlabVcsConnector.test.ts

@@ -9,6 +9,21 @@ import { GitLabVcsConnector } from "../../src/vcs/gitlabVcsConnector.js";
 import type { GitLabVcsConnectorConfig } from "../../src/vcs/gitlabVcsConnector.js";
 import { ReviewApiError } from "../../src/interfaces.js";
 
+const { logger } = vi.hoisted(() => ({
+  logger: {
+    trace: vi.fn(),
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    fatal: vi.fn(),
+  },
+}));
+
+vi.mock("../../src/logger.js", () => ({
+  getLogger: vi.fn(() => logger),
+}));
+
 vi.mock("child_process");
 vi.mock("../../src/connectors/gitlabHttpClient.js", () => {
   const GitLabHttpClient = vi.fn().mockImplementation(function () {
@@ -59,6 +74,25 @@ describe("GitLabVcsConnector", () => {
       );
     });
 
+    it("redacts credentials from clone logs without changing the Git command", async () => {
+      const mockExecFileSync = vi.mocked(execFileSync);
+      mockExecFileSync.mockReturnValue("");
+      const repoUrl =
+        "https://oauth2:glpat-secret@gitlab.example.com/my-project.git";
+
+      await connector.clone(repoUrl, "main", "/tmp/repo");
+
+      expect(mockExecFileSync).toHaveBeenCalledWith(
+        "git",
+        ["clone", "--branch", "main", "--depth", "1", repoUrl, "/tmp/repo"],
+        expect.any(Object)
+      );
+      expect(logger.info).toHaveBeenCalledWith(
+        expect.objectContaining({ repoUrl: "https://<redacted>@gitlab.example.com/my-project.git" }),
+        "cloning repository from GitLab via HTTP"
+      );
+    });
+
     it("should throw on clone failure", async () => {
       const mockExecFileSync = vi.mocked(execFileSync);
       mockExecFileSync.mockImplementation(() => {
@@ -69,6 +103,17 @@ describe("GitLabVcsConnector", () => {
         connector.clone("https://invalid.com/repo.git", "main", "/tmp/repo")
       ).rejects.toThrow("Failed to clone GitLab repository");
     });
+
+    it("redacts credentials echoed in clone failures", async () => {
+      const repoUrl = "https://oauth2:glpat-secret@gitlab.example.com/my-project.git";
+      vi.mocked(execFileSync).mockImplementationOnce(() => {
+        throw new Error(`fatal: unable to access '${repoUrl}'`);
+      });
+
+      await expect(connector.clone(repoUrl, "main", "/tmp/repo")).rejects.not.toThrow(
+        /glpat-secret/
+      );
+    });
   });
 
   describe("push", () => {

tests/unit/redactUrl.test.ts

@@ -0,0 +1,86 @@
+import { describe, it, expect } from "vitest";
+import { redactUrls, redactDockerArgs } from "../../src/utils/redactUrl.js";
+
+describe("redactUrls", () => {
+  it("masks credentials embedded in an https clone URL", () => {
+    const url =
+      "https://x-access-token:gho_ABCDEFGHIJKLMNOPQRST@github.com/savoirfairelinux/virtual-engineer.git";
+    const out = redactUrls(url);
+    expect(out).toBe("https://<redacted>@github.com/savoirfairelinux/virtual-engineer.git");
+    expect(out).not.toContain("gho_");
+  });
+
+  it("leaves credential-free URLs untouched", () => {
+    const url = "https://github.com/savoirfairelinux/virtual-engineer.git";
+    expect(redactUrls(url)).toBe(url);
+  });
+
+  it("handles uppercase schemes without redacting at-signs in URL paths", () => {
+    expect(redactUrls("HTTPS://user:secret@example.com/org/repo.git")).toBe(
+      "HTTPS://<redacted>@example.com/org/repo.git"
+    );
+    expect(redactUrls("https://example.com/users/dev@example.com")).toBe(
+      "https://example.com/users/dev@example.com"
+    );
+  });
+
+  it("masks GitHub tokens outside URL userinfo", () => {
+    const text =
+      "fatal: unable to access 'https://github.com/org/repo.git?access_token=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345'";
+    const out = redactUrls(text);
+    expect(out).not.toContain("ghp_");
+    expect(out).toContain("access_token=<redacted>");
+  });
+});
+
+describe("redactDockerArgs", () => {
+  it("masks the value of sensitive env-var assignments", () => {
+    const args = [
+      "run",
+      "--rm",
+      "-e",
+      "HOME=/ve-home",
+      "-e",
+      "GITHUB_TOKEN=ghu_ABCDEFGHIJKLMNOPQRSTUVWXYZ123456",
+      "-e",
+      "COPILOT_SDK_AUTH_TOKEN=gho_ABCDEFGHIJKLMNOPQRSTUVWXYZ123456",
+      "image",
+    ];
+    const out = redactDockerArgs(args);
+    expect(out).toContain("GITHUB_TOKEN=<redacted>");
+    expect(out).toContain("COPILOT_SDK_AUTH_TOKEN=<redacted>");
+    // Non-sensitive values stay readable.
+    expect(out).toContain("HOME=/ve-home");
+    expect(out.join(" ")).not.toContain("ghu_");
+    expect(out.join(" ")).not.toContain("gho_");
+  });
+
+  it("masks credentials embedded in URL-bearing args", () => {
+    const args = [
+      "git",
+      "clone",
+      "https://x-access-token:gho_ABCDEFGHIJKLMNOPQRST@github.com/org/repo.git",
+      "/workspace",
+    ];
+    const out = redactDockerArgs(args);
+    expect(out.join(" ")).not.toContain("gho_");
+    expect(out.join(" ")).toContain("<redacted>@github.com/org/repo.git");
+  });
+
+  it("masks stray token-shaped values even without a sensitive key name", () => {
+    const out = redactDockerArgs(["-e", "FOO=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345"]);
+    expect(out.join(" ")).not.toContain("ghp_");
+    expect(out.join(" ")).toContain("<redacted>");
+  });
+
+  it("does not mask benign env-var names containing sensitive substrings", () => {
+    const args = ["AUTHOR_NAME=alice", "TOKENIZER_MODE=strict", "PATH=/usr/bin"];
+    expect(redactDockerArgs(args)).toEqual(args);
+  });
+
+  it("masks sensitive assignments in long Docker env syntax", () => {
+    expect(redactDockerArgs(["--env=GITHUB_TOKEN=opaque-value"])).toEqual([
+      "--env=GITHUB_TOKEN=<redacted>",
+    ]);
+  });
+});