Skip to content

gstack-redact: pre-push guard is opt-in (installed by nothing), fails open on git errors, and misses common token types (glpat-, Bearer, hf_, npm_, GCP JSON) #1946

Description

@josuediazflores

Summary

The redaction engine is described as a guard that prevents secrets from being pushed, but in its default state it provides much weaker protection than it appears to. Three distinct gaps, found during a security review of v1.57.7.0:

1. The pre-push hook is opt-in and installed by nothing — a plain git push scans nothing

  • bin/gstack-config:126redact_prepush_hook defaults to "false".
  • The only references to install-prepush-hook are the hook itself (bin/gstack-redact-prepush) and its installer subcommand (bin/gstack-redact). Nothing in setup (or any skill / postinstall) ever installs it.

Net: unless a user manually runs gstack-redact install-prepush-hook, git push runs no secret scan at all. /ship scans the PR/MR body text, but not the diff being pushed.

2. The pre-push hook fails OPEN on any git failure or oversized diff

bin/gstack-redact-prepush:38-41:

function git(args: string[]): string {
  const r = spawnSync("git", args, { encoding: "utf8", maxBuffer: 64 * 1024 * 1024 });
  return r.status === 0 ? (r.stdout ?? "") : "";
}

Any non-zero git exit — or a diff exceeding the 64 MiB maxBuffer (then status is null and stdout is truncated) — yields "". addedLinesFor returns "", the loop hits if (!added.trim()) continue;, and main() falls through to process.exit(0) → push allowed. There is no "couldn't scan → block" branch; the oversized-diff case is exactly where a large secret-bearing blob is most likely. (This is distinct from #1824, which fixed a fail-open in the --max-bytes oversize guard — this is the git-helper path.)

3. Coverage gaps — common token types have no pattern

lib/redact-patterns.ts has no detector for several secret types, including ones for tooling gstack itself drives (GitLab / glab):

  • GitLab glpat-… (and glptt-/gldt-) — missing, despite first-class GitLab support
  • Authorization: Bearer <token> headers — missing
  • HuggingFace hf_…, npm npm_…, DigitalOcean dop_v1_…missing
  • GCP service-account JSON ("private_key_id" / …@…gserviceaccount.com) — missing (pem.private_key only matches a literal -----BEGIN … PRIVATE KEY----- block, not JSON-escaped \n keys)

The only generic catch-all is env.kv (lib/redact-patterns.ts:344), which is MEDIUM tier (warn-only in the pre-push hook) and whose regex requires an UPPERCASE variable name with = assignment:

/^[ \t]*(?:export[ \t]+)?[A-Z][A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIALS?|DSN|AUTH|COOKIE|SESSION|PRIVATE)[ \t]*=.../

So api_key=… (lowercase), apiKey: "…" / password: … (YAML/JSON colon), and any keyword not in the list produce no finding. Even when env.kv does match (e.g. GITLAB_TOKEN=glpat-…), it's MEDIUM, so the pre-push hook warns but does not block (it blocks only on HIGH).

Impact

A user relying on the guard can push live secrets believing they're protected: by default the hook isn't installed; when installed, a git hiccup or large diff silently passes; and several common credential formats — including GitLab PATs and Bearer tokens — aren't detected, while the one generic net is warn-only and uppercase/=-only.

Suggested fix

  • Install the pre-push hook during setup (with consent), or adjust the docs so it isn't presented as enforcement. At minimum, have /ship scan the pushed diff, not just the PR body.
  • Fail closed: if spawnSync returns non-zero, status === null, or truncated output, block the push with a clear "couldn't scan diff safely" message plus an explicit override env var. Distinguish "scanned, clean" from "couldn't scan."
  • Add patterns for the GitLab glpat- family, Authorization: Bearer …, hf_, npm_, dop_v1_, and GCP service-account JSON; add a case-insensitive + :-assignment variant of env.kv, entropy-gated to control false positives.

The core engine itself is solid (linear-time/ReDoS-safe, fails closed on a single oversized input, hash-only audit log) — this is about the wiring and coverage around it.


Cross-checked against existing issues (#1324, #1706, #1824, #1866) to avoid duplicates.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions