mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(redact): close the remaining #1946 fail-opens — detection coverage + one-time consent
Two of #1946's reported gaps were still open after the v1.64 fail-closed work (the git-error and oversized-diff paths in bin/gstack-redact-prepush are already strict, chunked, and pinned by tests): 1. Detection fail-open: env.kv required an UPPERCASE name with an '=' assignment, so 'api_key=…', 'apiKey: "…"', and 'password: …' — the most common real config shapes — produced NO finding at all. The pattern is now case-insensitive, accepts ':' (YAML/JSON) as well as '=' assignment, and handles quoted JSON keys. It stays MEDIUM and entropy-gated per the calibration rule (a generic net that cries wolf gets bypassed), with pinned cases for each closed shape plus the placeholder/entropy negatives. 2. Install fail-open: nothing ever offered the guard, so a plain 'git push' scanned nothing and users believing themselves protected weren't. setup now asks ONCE for consent on a real interactive terminal (maintainer decision 6): an explicit answer is recorded to the existing redact_prepush_hook key and never re-asked; a timeout or non-interactive run changes nothing and keeps the hint-only posture. Default stays FALSE, and setup still never installs the hook itself — /ship owns the per-repo install (the wrong-repo invariant is pinned by the existing 'setup carries the hint only' test). Tests: per-shape pattern cases, prompt gating statics (key-absence + TTY + timed default-N read), timeout-persists-nothing, non-interactive stays hint-only with no key write, and recorded-answer-is-silent behavior runs. Contributes to #1946 (the pre-push guard's fail-closed scan paths landed in earlier releases; this closes the coverage and consent gaps it names). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
9af589bb73
commit
9c0de5fed1
+11
-2
@@ -504,8 +504,17 @@ export const PATTERNS: RedactPattern[] = [
|
||||
id: "env.kv",
|
||||
tier: "MEDIUM",
|
||||
category: "secret",
|
||||
description: "Env-style SECRET assignment with high-entropy value",
|
||||
regex: /^[ \t]*(?:export[ \t]+)?[A-Z][A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)[ \t]*=[ \t]*['"]?([^\s'"]{8,})['"]?/,
|
||||
description: "Secret-named assignment (env/YAML/JSON) with high-entropy value",
|
||||
// #1946 gap 3: the original shape required an UPPERCASE name and an `=`
|
||||
// assignment, so `api_key=…`, `apiKey: "…"`, and `password: …` (YAML/JSON
|
||||
// colon form) produced NO finding at all — a detection fail-open on the
|
||||
// most common config shapes. Now case-insensitive with `:` or `=`
|
||||
// assignment and optional quotes around the key (JSON). Still MEDIUM and
|
||||
// entropy-gated: this is the calibrated generic net, not a blocker.
|
||||
// The name part is `[A-Za-z0-9_.-]*` + suffix (zero-or-more prefix, not
|
||||
// one-or-more): a mandatory first char would swallow the suffix's own
|
||||
// first letter and bare names like `password:` / `key:` would never match.
|
||||
regex: /^[ \t]*(?:export[ \t]+)?["']?[A-Za-z0-9_.-]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)["']?[ \t]*[:=][ \t]*["']?([^\s'"]{8,})["']?/i,
|
||||
// Only fire on high-entropy values — kills `FOO_KEY=changeme` FPs.
|
||||
validate: (span) =>
|
||||
!isPlaceholderSpan(span) &&
|
||||
|
||||
@@ -2002,19 +2002,47 @@ if [ "$NO_TEAM_MODE" -eq 1 ] && [ -x "$SETTINGS_HOOK" ]; then
|
||||
"$SETTINGS_HOOK" remove-source --source plan-tune-cathedral 2>/dev/null || true
|
||||
fi
|
||||
|
||||
# ─── Redact pre-push guard hint (#1946) ──────────────────────────────────────
|
||||
# ─── Redact pre-push guard consent (#1946) ───────────────────────────────────
|
||||
# The credential pre-push hook is per-REPO state — setup runs in the gstack
|
||||
# checkout, the wrong repo to install it into. /ship offers the install once
|
||||
# at the moment of relevance (first push) and silently installs in any repo
|
||||
# where redact_prepush_hook=true. This hint is setup's whole involvement.
|
||||
# Hint only when UNSET — an explicit "false" is a recorded decline and must
|
||||
# not be re-nagged on every setup run (adversarial review finding 11).
|
||||
# checkout, the wrong repo to install it into, so setup NEVER installs the
|
||||
# hook itself. /ship installs it silently in any repo where
|
||||
# redact_prepush_hook=true. What setup owns is CONSENT: on a real interactive
|
||||
# terminal it asks ONCE whether pushes should be scanned, recording the
|
||||
# answer to the existing redact_prepush_hook key (default stays false — a
|
||||
# timeout or non-interactive run changes nothing and keeps the hint-only
|
||||
# posture). An explicit answer is persisted and never re-asked; an explicit
|
||||
# "false" is a recorded decline (adversarial review finding 11).
|
||||
# `gstack-config get` defaults absent keys to "false", which is
|
||||
# indistinguishable from a decline — test key presence in the config file.
|
||||
_GSTACK_CFG_FILE="${GSTACK_HOME:-$HOME/.gstack}/config.yaml"
|
||||
if ! grep -q '^redact_prepush_hook:' "$_GSTACK_CFG_FILE" 2>/dev/null; then
|
||||
log ""
|
||||
log "Tip: gstack can block pushes containing credentials (per-repo git hook)."
|
||||
log " Enable once: gstack-config set redact_prepush_hook true — /ship"
|
||||
log " installs the hook automatically in every repo you ship from."
|
||||
if [ "$QUIET" -ne 1 ] && [ -t 0 ] && [ -t 1 ]; then
|
||||
_REDACT_PROMPT_TIMEOUT=10
|
||||
log ""
|
||||
log "Credential push guard: gstack can block pushes containing credentials"
|
||||
log "(a per-repo git pre-push hook; /ship installs it automatically in every"
|
||||
log "repo you ship from — nothing is installed right now)."
|
||||
printf "Enable the pre-push credential guard? [y/N] (default: N, auto-skips in %ss): " "$_REDACT_PROMPT_TIMEOUT"
|
||||
read -t "$_REDACT_PROMPT_TIMEOUT" -r _REDACT_REPLY </dev/tty 2>/dev/null || _REDACT_REPLY=""
|
||||
case "$_REDACT_REPLY" in
|
||||
y|Y)
|
||||
"$GSTACK_CONFIG" set redact_prepush_hook true 2>/dev/null || true
|
||||
log "Enabled. /ship will install the guard in each repo at first push."
|
||||
;;
|
||||
n|N)
|
||||
"$GSTACK_CONFIG" set redact_prepush_hook false 2>/dev/null || true
|
||||
log "Declined — recorded. Re-enable anytime: gstack-config set redact_prepush_hook true"
|
||||
;;
|
||||
*)
|
||||
# Timed out / empty: don't persist a decline — hint and ask next time.
|
||||
log ""
|
||||
log "Skipped for now. Enable anytime: gstack-config set redact_prepush_hook true"
|
||||
;;
|
||||
esac
|
||||
else
|
||||
log ""
|
||||
log "Tip: gstack can block pushes containing credentials (per-repo git hook)."
|
||||
log " Enable once: gstack-config set redact_prepush_hook true — /ship"
|
||||
log " installs the hook automatically in every repo you ship from."
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -166,6 +166,29 @@ describe("MEDIUM demoted credential-shaped patterns (TENSION-1)", () => {
|
||||
expect(ids("API_KEY=${MY_VAR}")).not.toContain("env.kv");
|
||||
});
|
||||
|
||||
// #1946 gap 3: the uppercase-`=`-only shape made lowercase and YAML/JSON
|
||||
// colon assignments invisible — the exact config shapes people actually
|
||||
// push. Each closed detection fail-open gets a pinned case.
|
||||
test("env.kv fires on lowercase = assignment (#1946)", () => {
|
||||
expect(ids("api_key=8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ")).toContain("env.kv");
|
||||
});
|
||||
test("env.kv fires on YAML colon assignment (#1946)", () => {
|
||||
expect(ids("password: 8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ")).toContain("env.kv");
|
||||
});
|
||||
test("env.kv fires on quoted JSON key colon assignment (#1946)", () => {
|
||||
expect(ids('"apiKey": "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ"')).toContain("env.kv");
|
||||
});
|
||||
test("env.kv colon/lowercase forms stay entropy-gated and placeholder-safe", () => {
|
||||
expect(ids("password: changeme")).not.toContain("env.kv");
|
||||
expect(ids("apiKey: YOUR_API_KEY_HERE")).not.toContain("env.kv");
|
||||
expect(ids("api_key=${MY_VAR}")).not.toContain("env.kv");
|
||||
});
|
||||
test("env.kv stays MEDIUM (calibration: generic net, not a blocker)", () => {
|
||||
const f = scan("api_key=8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ", { repoVisibility: "private" })
|
||||
.findings.find((x) => x.id === "env.kv");
|
||||
expect(f?.tier).toBe("MEDIUM");
|
||||
});
|
||||
|
||||
// #1946 — Bearer is the most FP-prone shape in the wave: docs and examples
|
||||
// are full of "Authorization: Bearer <token>". MEDIUM + header proximity +
|
||||
// the env.kv entropy recipe keep it calibrated.
|
||||
|
||||
@@ -221,6 +221,82 @@ describe("install UX surfaces (#1946 / eng review D3+D10)", () => {
|
||||
expect(tmpl).toContain(".redact-prepush-prompted");
|
||||
expect(tmpl).toContain("redact_prepush_hook");
|
||||
});
|
||||
|
||||
// #1946 / maintainer decision 6: setup asks ONCE for consent on a real TTY,
|
||||
// records the answer to the existing redact_prepush_hook key, and keeps the
|
||||
// hint-only posture everywhere else. Default stays FALSE; setup never
|
||||
// installs the hook itself (the assertion above pins that).
|
||||
describe("one-time consent prompt in setup (#1946, decision 6)", () => {
|
||||
const setup = fs.readFileSync(path.join(ROOT, "setup"), "utf8");
|
||||
const block = setup.slice(setup.indexOf("# ─── Redact pre-push guard consent"));
|
||||
|
||||
test("prompt is gated on key ABSENCE and a real TTY, with a timed default-N read", () => {
|
||||
expect(block).toContain("grep -q '^redact_prepush_hook:'");
|
||||
expect(block).toContain('[ -t 0 ] && [ -t 1 ]');
|
||||
expect(block).toContain("[y/N]");
|
||||
expect(block).toContain('read -t "$_REDACT_PROMPT_TIMEOUT"');
|
||||
});
|
||||
|
||||
test("an explicit answer persists true/false; timeout persists NOTHING", () => {
|
||||
expect(block).toContain("set redact_prepush_hook true");
|
||||
expect(block).toContain("set redact_prepush_hook false");
|
||||
// The timeout branch must not write the key (a silent decline would
|
||||
// permanently suppress the ask without the user ever seeing it). The
|
||||
// branch's hint TEXT mentions the command; the executable invocation is
|
||||
// the quoted "$GSTACK_CONFIG" form.
|
||||
const timeoutBranch = block.slice(block.indexOf("*)"), block.indexOf("esac"));
|
||||
expect(timeoutBranch).not.toContain('"$GSTACK_CONFIG" set redact_prepush_hook');
|
||||
});
|
||||
|
||||
test("non-interactive setup keeps the hint-only posture (no prompt, no key write)", () => {
|
||||
const home = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-consent-"));
|
||||
try {
|
||||
const script = [
|
||||
"QUIET=0",
|
||||
'log() { echo "$@"; }',
|
||||
`GSTACK_CONFIG="${path.join(ROOT, "bin", "gstack-config")}"`,
|
||||
block,
|
||||
].join("\n");
|
||||
const r = spawnSync("bash", ["-c", script], {
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "pipe"], // stdin not a TTY
|
||||
env: { ...process.env, GSTACK_HOME: home },
|
||||
timeout: 15_000,
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain("Tip:");
|
||||
expect(r.stdout).not.toContain("[y/N]");
|
||||
const cfg = path.join(home, "config.yaml");
|
||||
const cfgText = fs.existsSync(cfg) ? fs.readFileSync(cfg, "utf8") : "";
|
||||
expect(cfgText).not.toContain("redact_prepush_hook");
|
||||
} finally {
|
||||
fs.rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("a recorded answer is never re-asked (key present → silent)", () => {
|
||||
const home = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-consent-set-"));
|
||||
try {
|
||||
fs.writeFileSync(path.join(home, "config.yaml"), "redact_prepush_hook: false\n");
|
||||
const script = [
|
||||
"QUIET=0",
|
||||
'log() { echo "$@"; }',
|
||||
`GSTACK_CONFIG="${path.join(ROOT, "bin", "gstack-config")}"`,
|
||||
block,
|
||||
].join("\n");
|
||||
const r = spawnSync("bash", ["-c", script], {
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env, GSTACK_HOME: home },
|
||||
timeout: 15_000,
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout.trim()).toBe("");
|
||||
} finally {
|
||||
fs.rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("escape valve", () => {
|
||||
|
||||
Reference in New Issue
Block a user