mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-11 15:39:04 +02:00
fix: adversarial review fixes (Claude + Codex cross-model passes)
Both adversarial passes ran against the wave; every FIXABLE finding landed with a regression test: - probeTimeoutMs clamps to >=1ms: a fractional override floored to 0, and execFileSync treats timeout:0 as NO timeout — the probe that exists to bound hangs could hang forever (found by both models independently). - /ship silent hook install now requires the hooks dir to live inside .git: with core.hooksPath (husky's COMMITTED .husky/), the chaining installer would have renamed the team's committed pre-push and written a machine-local wrapper into the working tree (found by both models). - gstack-config gbrain-refresh accepts the "timeout" status — the last consumer still gating on literal "ok" (Codex); gstack-gbrain-detect's config-derived fields honor GBRAIN_HOME so the detection JSON can't report status ok alongside config_exists false (Codex). - prepush: a remote sha absent locally (shallow clone / stale fetch) falls back to the merge-base/empty-tree range — scans MORE, never blocks a legitimate push into training users toward --no-verify. - dashboards: curl's own 000 no longer doubles to "HTTP 000000"; the community dashboard flags stale snapshots like the security one; array sections parse via jq (the sed/grep loops truncated at the first ']'); the no-jq marker grep tolerates whitespace. - telemetry: multi-line redactor output nulls the field instead of corrupting the JSONL record; setup's hint fires only when the config key is genuinely unset (an explicit false is a recorded decline); the /ship prompt marker honors GSTACK_HOME. Kept as designed (cross-model tension noted): Bearer stays MEDIUM in the prepush gate — a HIGH Bearer would block every docs example; the entropy validator can't eliminate that FP class, and MEDIUM warns visibly. 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
8c8e3b9e52
commit
201e46b230
+18
-4
@@ -1225,19 +1225,33 @@ _REDACT_PREPUSH=$(~/.claude/skills/gstack/bin/gstack-config get redact_prepush_h
|
||||
_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "")
|
||||
_HOOK_INSTALLED="no"
|
||||
[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes"
|
||||
_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no")
|
||||
# Custom hooks dirs (core.hooksPath — e.g. husky's COMMITTED .husky/) must
|
||||
# never get a silent install: the chaining installer would rename the team's
|
||||
# committed hook and write a machine-local wrapper into the working tree.
|
||||
_HOOKS_DIR=$(git rev-parse --git-path hooks 2>/dev/null || echo "")
|
||||
_GIT_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || echo "")
|
||||
_HOOKS_IN_GIT_DIR="no"
|
||||
case "$_HOOKS_DIR" in
|
||||
"$_GIT_DIR"/*|hooks|.git/hooks) _HOOKS_IN_GIT_DIR="yes" ;;
|
||||
esac
|
||||
_PREPUSH_PROMPTED=$([ -f "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted" ] && echo "yes" || echo "no")
|
||||
echo "REDACT_PREPUSH: $_REDACT_PREPUSH"
|
||||
echo "HOOK_INSTALLED: $_HOOK_INSTALLED"
|
||||
echo "HOOKS_IN_GIT_DIR: $_HOOKS_IN_GIT_DIR"
|
||||
echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED"
|
||||
```
|
||||
|
||||
Branch on the echoed values:
|
||||
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already
|
||||
given; install silently (no question) and continue:
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no` and `HOOKS_IN_GIT_DIR: yes`** —
|
||||
consent already given; install silently (no question) and continue:
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-redact install-prepush-hook
|
||||
```
|
||||
If `HOOKS_IN_GIT_DIR: no` (husky or another committed hooks dir), do NOT
|
||||
install silently — print one line: "redact pre-push guard not installed:
|
||||
this repo uses a custom core.hooksPath; run
|
||||
`gstack-redact install-prepush-hook` manually if you want it chained."
|
||||
2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time
|
||||
offer (fires once EVER, machine-wide). AskUserQuestion:
|
||||
|
||||
@@ -1256,7 +1270,7 @@ Branch on the echoed values:
|
||||
ALWAYS (after either answer, but NOT if the question itself failed to
|
||||
render — a failed AskUserQuestion must re-offer next time):
|
||||
```bash
|
||||
touch ~/.gstack/.redact-prepush-prompted
|
||||
touch "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted"
|
||||
```
|
||||
3. **Anything else** (declined earlier, or already installed) — continue
|
||||
without comment.
|
||||
|
||||
+18
-4
@@ -2392,19 +2392,33 @@ _REDACT_PREPUSH=$($GSTACK_ROOT/bin/gstack-config get redact_prepush_hook 2>/dev/
|
||||
_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "")
|
||||
_HOOK_INSTALLED="no"
|
||||
[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes"
|
||||
_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no")
|
||||
# Custom hooks dirs (core.hooksPath — e.g. husky's COMMITTED .husky/) must
|
||||
# never get a silent install: the chaining installer would rename the team's
|
||||
# committed hook and write a machine-local wrapper into the working tree.
|
||||
_HOOKS_DIR=$(git rev-parse --git-path hooks 2>/dev/null || echo "")
|
||||
_GIT_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || echo "")
|
||||
_HOOKS_IN_GIT_DIR="no"
|
||||
case "$_HOOKS_DIR" in
|
||||
"$_GIT_DIR"/*|hooks|.git/hooks) _HOOKS_IN_GIT_DIR="yes" ;;
|
||||
esac
|
||||
_PREPUSH_PROMPTED=$([ -f "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted" ] && echo "yes" || echo "no")
|
||||
echo "REDACT_PREPUSH: $_REDACT_PREPUSH"
|
||||
echo "HOOK_INSTALLED: $_HOOK_INSTALLED"
|
||||
echo "HOOKS_IN_GIT_DIR: $_HOOKS_IN_GIT_DIR"
|
||||
echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED"
|
||||
```
|
||||
|
||||
Branch on the echoed values:
|
||||
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already
|
||||
given; install silently (no question) and continue:
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no` and `HOOKS_IN_GIT_DIR: yes`** —
|
||||
consent already given; install silently (no question) and continue:
|
||||
```bash
|
||||
$GSTACK_ROOT/bin/gstack-redact install-prepush-hook
|
||||
```
|
||||
If `HOOKS_IN_GIT_DIR: no` (husky or another committed hooks dir), do NOT
|
||||
install silently — print one line: "redact pre-push guard not installed:
|
||||
this repo uses a custom core.hooksPath; run
|
||||
`gstack-redact install-prepush-hook` manually if you want it chained."
|
||||
2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time
|
||||
offer (fires once EVER, machine-wide). AskUserQuestion:
|
||||
|
||||
@@ -2423,7 +2437,7 @@ Branch on the echoed values:
|
||||
ALWAYS (after either answer, but NOT if the question itself failed to
|
||||
render — a failed AskUserQuestion must re-offer next time):
|
||||
```bash
|
||||
touch ~/.gstack/.redact-prepush-prompted
|
||||
touch "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted"
|
||||
```
|
||||
3. **Anything else** (declined earlier, or already installed) — continue
|
||||
without comment.
|
||||
|
||||
+18
-4
@@ -2798,19 +2798,33 @@ _REDACT_PREPUSH=$($GSTACK_ROOT/bin/gstack-config get redact_prepush_hook 2>/dev/
|
||||
_HOOK_PATH=$(git rev-parse --git-path hooks/pre-push 2>/dev/null || echo "")
|
||||
_HOOK_INSTALLED="no"
|
||||
[ -n "$_HOOK_PATH" ] && [ -f "$_HOOK_PATH" ] && grep -q "gstack-redact" "$_HOOK_PATH" 2>/dev/null && _HOOK_INSTALLED="yes"
|
||||
_PREPUSH_PROMPTED=$([ -f ~/.gstack/.redact-prepush-prompted ] && echo "yes" || echo "no")
|
||||
# Custom hooks dirs (core.hooksPath — e.g. husky's COMMITTED .husky/) must
|
||||
# never get a silent install: the chaining installer would rename the team's
|
||||
# committed hook and write a machine-local wrapper into the working tree.
|
||||
_HOOKS_DIR=$(git rev-parse --git-path hooks 2>/dev/null || echo "")
|
||||
_GIT_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || echo "")
|
||||
_HOOKS_IN_GIT_DIR="no"
|
||||
case "$_HOOKS_DIR" in
|
||||
"$_GIT_DIR"/*|hooks|.git/hooks) _HOOKS_IN_GIT_DIR="yes" ;;
|
||||
esac
|
||||
_PREPUSH_PROMPTED=$([ -f "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted" ] && echo "yes" || echo "no")
|
||||
echo "REDACT_PREPUSH: $_REDACT_PREPUSH"
|
||||
echo "HOOK_INSTALLED: $_HOOK_INSTALLED"
|
||||
echo "HOOKS_IN_GIT_DIR: $_HOOKS_IN_GIT_DIR"
|
||||
echo "PREPUSH_PROMPTED: $_PREPUSH_PROMPTED"
|
||||
```
|
||||
|
||||
Branch on the echoed values:
|
||||
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no`** — consent already
|
||||
given; install silently (no question) and continue:
|
||||
1. **`REDACT_PREPUSH: true` and `HOOK_INSTALLED: no` and `HOOKS_IN_GIT_DIR: yes`** —
|
||||
consent already given; install silently (no question) and continue:
|
||||
```bash
|
||||
$GSTACK_ROOT/bin/gstack-redact install-prepush-hook
|
||||
```
|
||||
If `HOOKS_IN_GIT_DIR: no` (husky or another committed hooks dir), do NOT
|
||||
install silently — print one line: "redact pre-push guard not installed:
|
||||
this repo uses a custom core.hooksPath; run
|
||||
`gstack-redact install-prepush-hook` manually if you want it chained."
|
||||
2. **`REDACT_PREPUSH` not true AND `PREPUSH_PROMPTED: no`** — one-time
|
||||
offer (fires once EVER, machine-wide). AskUserQuestion:
|
||||
|
||||
@@ -2829,7 +2843,7 @@ Branch on the echoed values:
|
||||
ALWAYS (after either answer, but NOT if the question itself failed to
|
||||
render — a failed AskUserQuestion must re-offer next time):
|
||||
```bash
|
||||
touch ~/.gstack/.redact-prepush-prompted
|
||||
touch "${GSTACK_HOME:-$HOME/.gstack}/.redact-prepush-prompted"
|
||||
```
|
||||
3. **Anything else** (declined earlier, or already installed) — continue
|
||||
without comment.
|
||||
|
||||
@@ -43,7 +43,10 @@ function runDetect(env: Partial<NodeJS.ProcessEnv>): string {
|
||||
encoding: "utf-8",
|
||||
timeout: 15_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env, ...env },
|
||||
// GBRAIN_HOME pinned empty: detect honors it (codex D11), and sibling
|
||||
// test files in the same shard set it ambiently — without the pin, the
|
||||
// spawned detect reads the polluter's (or the developer's real) config.
|
||||
env: { ...process.env, GBRAIN_HOME: "", ...env },
|
||||
});
|
||||
}
|
||||
|
||||
@@ -52,7 +55,7 @@ function runIsOk(env: Partial<NodeJS.ProcessEnv>): number {
|
||||
const r = spawnSync(BUN_BIN, ["run", DETECT_BIN, "--is-ok"], {
|
||||
timeout: 15_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env, ...env },
|
||||
env: { ...process.env, GBRAIN_HOME: "", ...env },
|
||||
});
|
||||
return r.status ?? 1;
|
||||
}
|
||||
|
||||
@@ -297,6 +297,11 @@ describe("probeTimeoutMs — env override parsing", () => {
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "0" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "-5" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
});
|
||||
|
||||
it("never returns 0 for fractional sub-millisecond values (0 = NO timeout in execFileSync)", () => {
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "0.5" })).toBe(1);
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "0.0001" })).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("lib/gbrain-local-status — cache behavior", () => {
|
||||
|
||||
@@ -128,6 +128,19 @@ describe("fail closed on unscannable diffs (#1946)", () => {
|
||||
expect(code).toBe(0);
|
||||
});
|
||||
|
||||
test("a remote sha absent locally (shallow clone / stale fetch) falls back to scanning MORE, not blocking", () => {
|
||||
// Adversarial review finding 8: remote..local can't resolve when the
|
||||
// remote tip object isn't in the local odb. The fallback scans the
|
||||
// merge-base/empty-tree range — a secret in the pushed content still
|
||||
// blocks; a clean push passes instead of hard-failing.
|
||||
const fakeRemoteSha = "c".repeat(40);
|
||||
const head = commit("secrets.txt", "key AKIA1234567890ABCDEF\n", "leaky commit");
|
||||
const { code, stderr } = runHook(`refs/heads/main ${head} refs/heads/main ${fakeRemoteSha}\n`);
|
||||
expect(code).toBe(1); // fallback range still catches the credential
|
||||
expect(stderr).toContain("aws.access_key");
|
||||
expect(stderr).not.toContain("could not compute the pushed diff");
|
||||
});
|
||||
|
||||
test("a diff killed by a signal (null status — the maxBuffer/kill class) BLOCKS", () => {
|
||||
// Stub git: probes delegate to the real git; the diff invocation kills
|
||||
// itself, producing spawnSync status === null. This is the exact branch
|
||||
|
||||
@@ -101,7 +101,7 @@ function run(
|
||||
// needs — everything except jq.
|
||||
const toolBin = join(tmp, "tool-bin");
|
||||
mkdirSync(toolBin, { recursive: true });
|
||||
for (const tool of ["mktemp", "cat", "grep", "head", "sed", "awk", "rm", "sh", "bash"]) {
|
||||
for (const tool of ["mktemp", "cat", "grep", "head", "sed", "awk", "rm", "sh", "bash", "tr", "tail"]) {
|
||||
const real = Bun.which(tool);
|
||||
if (real) symlinkSync(real, join(toolBin, tool));
|
||||
}
|
||||
@@ -254,4 +254,19 @@ describe("gstack-community-dashboard — never reports fake zeros (#1947)", () =
|
||||
expect(r.stdout).toContain("Weekly active installs: 42");
|
||||
expect(r.stdout).not.toContain("unverified");
|
||||
});
|
||||
|
||||
it("stale snapshot flagged in human mode (matches security-dashboard)", () => {
|
||||
const staleBody = JSON.stringify({ ...JSON.parse(GOOD_BODY_MARKER), stale: true });
|
||||
const r = run(COMM_BIN, { mode: "ok", body: staleBody });
|
||||
expect(r.stdout).toContain("Weekly active installs: 42");
|
||||
expect(r.stdout).toContain("stale snapshot");
|
||||
});
|
||||
|
||||
it("network failure reports HTTP 000, never a doubled 000000", () => {
|
||||
// Adversarial review finding 6: curl prints its own 000 before a
|
||||
// non-zero exit; a `|| echo` doubled it in user-facing output.
|
||||
const r = run(COMM_BIN, { mode: "netfail" });
|
||||
expect(r.stdout).toContain("(HTTP 000)");
|
||||
expect(r.stdout).not.toContain("000000");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user