pullfrog · theoephraim · Jul 28, 2026
diff --git a/main.ts b/main.ts
@@ -237,6 +237,9 @@ export async function main(): Promise<MainResult> {
   // sanitizeSecret trims + masks so accidental trailing whitespace doesn't leak
   // through GitHub Actions' line-based log masking. whitespace-only values
   // return null and skip injection so the user sees a clear missing-key error.
+  // this channel also carries non-credential config (model ids, regions,
+  // locations); sanitizeSecret trims those but skips masking, since masking is
+  // by value and would blank out unrelated log text. see isNonSecretConfigName.
   if (runContext.dbSecrets) {
     for (const [key, value] of Object.entries(runContext.dbSecrets)) {
       if (!process.env[key]) {

diff --git a/utils/normalizeEnv.test.ts b/utils/normalizeEnv.test.ts
@@ -1,15 +1,24 @@
-import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import * as core from "@actions/core";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { normalizeEnv, sanitizeSecret } from "./normalizeEnv.ts";
+import { isSensitiveEnvName } from "./secrets.ts";
 
 /**
  * These tests pin the load-bearing invariants of secret sanitisation:
  *   - sensitive values are trimmed before downstream code reads them
  *   - whitespace-only values are NOT silently zeroed (leave env unchanged)
  *   - case normalisation still happens
+ *   - masking is applied to everything except an explicit config allowlist
  *
- * Masking (`core.setSecret`) is delegated to `@actions/core` and trusted to
- * work as documented — we don't spy on stdout to re-test the toolkit.
+ * We don't re-test what `core.setSecret` does with a value (that's the
+ * toolkit's job), but we do assert *whether* we call it: the decision of what
+ * counts as maskable is ours, and getting it wrong either leaks a credential
+ * or blanks out unrelated log text.
  */
+vi.mock("@actions/core", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("@actions/core")>()),
+  setSecret: vi.fn(),
+}));
 
 describe("normalizeEnv: process.env state contract", () => {
   let originalEnv: NodeJS.ProcessEnv;
@@ -101,3 +110,71 @@ describe("sanitizeSecret return value", () => {
     );
   });
 });
+
+describe("sanitizeSecret masking policy", () => {
+  const setSecret = vi.mocked(core.setSecret);
+
+  beforeEach(() => {
+    setSecret.mockClear();
+  });
+
+  it("masks credential-shaped keys", () => {
+    sanitizeSecret("ANTHROPIC_API_KEY", "sk-ant-secret");
+    expect(setSecret).toHaveBeenCalledWith("sk-ant-secret");
+  });
+
+  it("masks unrecognised keys — the allowlist fails closed", () => {
+    sanitizeSecret("SOME_FUTURE_PROVIDER_CREDS", "hunter2");
+    expect(setSecret).toHaveBeenCalledWith("hunter2");
+  });
+
+  it("masks VERTEX_SERVICE_ACCOUNT_JSON even though it matches no sensitive suffix", () => {
+    // regression guard for the tempting "just gate on isSensitiveEnvName"
+    // refactor: this key is a real credential protected *only* by
+    // mask-by-default, so narrowing the gate would silently unmask it.
+    expect(isSensitiveEnvName("VERTEX_SERVICE_ACCOUNT_JSON")).toBe(false);
+    sanitizeSecret("VERTEX_SERVICE_ACCOUNT_JSON", '{"private_key":"pk"}');
+    expect(setSecret).toHaveBeenCalledWith('{"private_key":"pk"}');
+  });
+
+  it("does not mask non-secret config values", () => {
+    // masking is by value: setSecret("global") would rewrite every unrelated
+    // occurrence of "global" in the run log to ***.
+    sanitizeSecret("VERTEX_LOCATION", "global");
+    expect(setSecret).not.toHaveBeenCalled();
+  });
+
+  it("still trims non-secret config values", () => {
+    // a trailing newline on a model id breaks the exact-match authorization
+    // lookup, so trimming has to happen whether or not we mask.
+    expect(sanitizeSecret("PULLFROG_MODEL", "azure/gpt-5.6-sol\n")).toBe("azure/gpt-5.6-sol");
+    expect(setSecret).not.toHaveBeenCalled();
+  });
+
+  it("matches config names case-insensitively", () => {
+    sanitizeSecret("aws_region", "us-east-1");
+    expect(setSecret).not.toHaveBeenCalled();
+  });
+
+  it("does not mask the Azure config values", () => {
+    // the console flow stores all five Azure values in the account-secret
+    // channel, and these carry the worst mask-by-value collateral: "128000"
+    // and "true" appear all over an ordinary run log.
+    sanitizeSecret("AZURE_RESOURCE_NAME", "my-resource");
+    sanitizeSecret("AZURE_DEPLOYMENT", "prod-reasoning");
+    sanitizeSecret("AZURE_CONTEXT", "400000");
+    sanitizeSecret("AZURE_MAX_OUTPUT", "128000");
+    sanitizeSecret("AZURE_USE_CHAT_COMPLETIONS", "true");
+    expect(setSecret).not.toHaveBeenCalled();
+  });
+
+  it("masks OPENAI_COMPATIBLE_BASE_URL even though its siblings are config", () => {
+    // gateway URLs can carry account ids or embedded credentials in the path,
+    // so the base URL stays off the allowlist while model/context/max-output
+    // are unmasked.
+    sanitizeSecret("OPENAI_COMPATIBLE_MODEL", "my-model");
+    expect(setSecret).not.toHaveBeenCalled();
+    sanitizeSecret("OPENAI_COMPATIBLE_BASE_URL", "https://gw.example.com/v1");
+    expect(setSecret).toHaveBeenCalledWith("https://gw.example.com/v1");
+  });
+});
diff --git a/utils/normalizeEnv.ts b/utils/normalizeEnv.ts
@@ -1,6 +1,6 @@
 import * as core from "@actions/core";
 import { log } from "./cli.ts";
-import { isSensitiveEnvName } from "./secrets.ts";
+import { isNonSecretConfigName, isSensitiveEnvName } from "./secrets.ts";
 
 /**
  * Trim surrounding whitespace from a sensitive value and register it as a
@@ -19,6 +19,13 @@ import { isSensitiveEnvName } from "./secrets.ts";
  * callers must leave `process.env` untouched in that case so a misconfigured
  * value surfaces as a clear "missing key" downstream rather than silently
  * mutating to the empty string.
+ *
+ * Keys in the non-secret config allowlist (`isNonSecretConfigName`) are still
+ * trimmed — a trailing newline on a model id breaks exact-match lookups just
+ * as badly as it breaks masking — but are NOT registered as masks. Masking is
+ * by value, so a config value like `global` or `us-east-1` would blank out
+ * unrelated log text. Everything not on that allowlist is masked, so an
+ * unrecognised key still fails closed.
  */
 /** C0 controls + DEL — the bytes an HTTP header value cannot carry. */
 export function hasControlCharacter(value: string): boolean {
@@ -52,9 +59,10 @@ export function sanitizeSecret(key: string, value: string): string | null {
   }
   if (trimmed !== value) {
     log.warning(
-      `» stripped whitespace from ${key} (whitespace in secret values breaks GitHub Actions log masking)`
+      `» stripped whitespace from ${key} (whitespace breaks exact-match lookups and GitHub Actions log masking)`
     );
   }
+  if (isNonSecretConfigName(key)) return trimmed;
   core.setSecret(trimmed);
   return trimmed;
 }

diff --git a/utils/secrets.ts b/utils/secrets.ts
@@ -23,6 +23,47 @@ export function isSensitiveEnvName(key: string): boolean {
   return SENSITIVE_PATTERNS.some((p) => p.test(key));
 }
 
+// Config-shaped vars that arrive through the same account-secret channel as real
+// credentials but carry no secret material: model specifiers, regions, project and
+// location identifiers. GitHub Actions masks by *value*, so masking these is
+// actively harmful — a short, common value (VERTEX_LOCATION=global) rewrites every
+// unrelated occurrence of that word in the run log to ***, and a masked model id
+// makes the "which model ran?" lines unreadable exactly when someone is debugging
+// why the wrong model ran.
+//
+// Deliberately an explicit allowlist rather than a suffix rule, so masking stays
+// fail-closed: an unrecognised key is still treated as a secret. In particular
+// VERTEX_SERVICE_ACCOUNT_JSON must NOT be added here — it matches none of
+// SENSITIVE_PATTERNS, so unconditional masking is the only thing protecting it.
+// OPENAI_COMPATIBLE_BASE_URL is also deliberately absent: gateway URLs can carry
+// account ids or embedded credentials in the path, so it stays masked.
+const NON_SECRET_CONFIG_NAMES = new Set([
+  "PULLFROG_MODEL",
+  "PULLFROG_AGENT",
+  "AWS_REGION",
+  "BEDROCK_MODEL_ID",
+  "VERTEX_MODEL_ID",
+  "VERTEX_LOCATION",
+  "GOOGLE_CLOUD_PROJECT",
+  // Azure OpenAI — everything but AZURE_API_KEY is plain config, and the
+  // console flow stores all of it in the account-secret channel. The limits
+  // ("128000") and the Chat Completions flag ("true") are the worst offenders
+  // if masked: those values appear all over an ordinary run log.
+  "AZURE_RESOURCE_NAME",
+  "AZURE_DEPLOYMENT",
+  "AZURE_CONTEXT",
+  "AZURE_MAX_OUTPUT",
+  "AZURE_USE_CHAT_COMPLETIONS",
+  // OpenAI-compatible — same shape, minus the base URL (see above).
+  "OPENAI_COMPATIBLE_MODEL",
+  "OPENAI_COMPATIBLE_CONTEXT",
+  "OPENAI_COMPATIBLE_MAX_OUTPUT",
+]);
+
+export function isNonSecretConfigName(key: string): boolean {
+  return NON_SECRET_CONFIG_NAMES.has(key.toUpperCase());
+}
+
 // --- subprocess env filtering ---
 
 // prefixes whose vars are safe to pass through (runner metadata, workflow context).