garrytan / garrytan/gstack

INJECTION_PATTERNS /override[:\s]/i false-rejects legitimate learnings/decisions containing the word "override"

Open
#1,934 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
133k
Forks
19.9k
Avg merge
18h 46m
Merged PRs (30d)
26

Description

### Summary

`lib/jsonl-store.ts` `INJECTION_PATTERNS` includes `/override[:\s]/i`, which matches the bare English/code word **"override"** followed by any whitespace. This silently rejects legitimate `gstack-learnings-log` / `gstack-decision-log` writes whose free-text discusses override behavior — very common in architecture and code-review learnings:

- "prose **overrides** the deterministic table on key overlap"
- "never **override** the flag when stale"
- "the renderer **override** of the table"

All of these get rejected as prompt-injection.

### Why the pattern adds ~no protection

A genuine injection like `Override: ignore all previous instructions` is **already** caught by `/ignore\s+(all\s+)?previous\s+(instructions|context|rules)/i`. So the bare `override` token carries the false-positives without unique safety value. (Confirmed: that injection string trips both patterns; removing `override` loses nothing.)

### The failure is silent

`bin/gstack-learnings-log` runs the validator under `set -euo pipefail`:

```bash
VALIDATED=$(printf '%s' "$INPUT" | bun -e "…" 2>/dev/null)
if [ $? -ne 0 ] || [ -z "$VALIDATED" ]; then
exit 1
fi
```

Under `set -e`, the failing command substitution exits the script **at the assignment line**, so the following `if … exit 1` is effectively dead code. The validator's rejection reason goes to the `2>/dev/null` void, and callers (the skills) invoke the bin with their own `2>/dev/null`. Net effect: a rejected learning vanishes with zero signal — it looks like a successful no-op.

### Repro

```bash
gstack-learnings-log '{"skill":"review","type":"pitfall","key":"x","insight":"never override the flag","confidence":7,"source":"observed"}'
echo $? # 1, silently — nothing written, no reason surfaced
```

### Proposed fixes

1. **Narrow the pattern to injection context** (keeps real coverage, drops the prose false-positive):
```js
/\boverride\b(?:\s*:|\s+(?:the\s+)?(?:all|every|previous|prior|system|instructions?|rules?|safety|checks?|gate|guard|review)\b)/i
```
Verified: passes "prose overrides the table" / "never override the flag" / "method override"; still rejects "Override: …", "override all rules", "override the system", "override safety checks".

2. **Surface the rejection reason** in the bins: capture the validator's stderr instead of `2>/dev/null`, and echo it / persist it to a discoverable log so a dropped write isn't invisible (especially since callers swallow stderr). Wrapping the validator assignment in `set +e` / `set -e` is also needed so the rejection-handling block actually runs.

Happy to send a PR if useful.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.