Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/scripts/classify.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,40 @@ try {
fail(`failed to read ${RULES_PATH}: ${e.message}`);
}

// A '[' in a pattern is a minimatch character class, never a literal
// bracket — a SvelteKit-style 'src/routes/[id]/+page.ts' entry silently
// matches nothing, and GitHub CODEOWNERS drops bracket lines entirely, so
// the gate the author thinks exists doesn't. That exact failure shipped
// once: wxa-jake-ai's stream gate was a no-op for six weeks
// (wxa-jake-ai#783). ALL brackets are banned, including intentional
// character classes like '*.[jt]s': '[id]' is syntactically a valid char
// class too, so no check can tell intent apart, and a heuristic that
// guesses wrong recreates the silent dead gate. Enumerate instead
// ('*.js' + '*.ts') — the 2026-07-04 fleet audit found zero real
// char-class uses, so strictness costs nothing. Fail closed.
//
// 'always_review' is validated here too even though this script never
// matches against it: codex-gate.mjs consumes it with the same minimatch
// semantics but is deliberately permissive on config errors, so this
// fail-closed pass is the only place a dead always_review entry gets
// caught before it silently skips a required Codex review.
for (const cls of [...PATTERN_CLASSES, 'always_review']) {
for (const p of rules[cls] || []) {
if (typeof p === 'string' && (p.includes('[') || p.includes(']'))) {
fail(
`${RULES_PATH}: pattern '${p}' (under '${cls}:') contains a bracket — ` +
`minimatch reads '[...]' as a character class, so a literal path like a SvelteKit ` +
`'[id]' segment can never match. Replace the bracket segment with '*' ` +
`(e.g. 'src/routes/api/chat/*/stream/+server.ts') or use a parent '**' glob, ` +
`and mirror the same fix in .github/CODEOWNERS (GitHub ignores bracket lines there). ` +
`If you meant a real character class like '*.[jt]s', enumerate it instead ` +
`('*.js' + '*.ts') — brackets are banned outright because intent is ambiguous. ` +
`Context: wxa-jake-ai#783.`
);
}
}
}

const changedFiles = readFileSync(0, 'utf8')
.split('\n')
.map((s) => s.trim())
Expand Down
114 changes: 114 additions & 0 deletions selftest/test_classify_bracket_guard.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
#!/usr/bin/env bash
# Guards classify.mjs's bracket-pattern validation.
#
# minimatch treats '[...]' as a character class, so a literal SvelteKit-style
# entry like 'src/routes/api/chat/[id]/stream/+server.ts' in risk-paths.yml
# can never match its own file — the gate is a silent no-op. GitHub
# CODEOWNERS additionally ignores any line containing brackets, so the
# lockstep CODEOWNERS mirror dies the same way. This exact failure shipped
# in wxa-jake-ai (stream gate dead 2026-05-24 → 2026-07-04, fixed in
# wxa-jake-ai#783; fleet audited same day — no other repo affected).
# classify.mjs now fails closed on bracket patterns; this test bakes that
# behavior AND the recommended '*' rewrite into version control.
#
# Run from the repo root:
# bash selftest/test_classify_bracket_guard.sh
set -euo pipefail

tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT

# classify.mjs resolves its deps from its own location, so install them next
# to a copy in the temp dir — same versions pr-classify.yml pins.
cp .github/scripts/classify.mjs "$tmp/classify.mjs"
(cd "$tmp" && npm install --no-save --silent yaml@2 minimatch@10 >/dev/null 2>&1)

mkdir -p "$tmp/repo/.github"
failed=0

# 1. A bracket pattern must fail closed with an actionable message.
cat > "$tmp/repo/.github/risk-paths.yml" <<'YAML'
blocked: []
sensitive:
- 'src/routes/api/chat/[id]/stream/+server.ts'
YAML
set +e
out=$(cd "$tmp/repo" && echo "README.md" | node "$tmp/classify.mjs" 2>&1)
rc=$?
set -e
if [ "$rc" -ne 0 ] && echo "$out" | grep -q "character class"; then
echo "✓ bracket pattern fails closed with an actionable message"
else
echo "✗ expected nonzero exit + 'character class' in message; got rc=$rc:"
echo "$out" | sed 's/^/ /'
failed=1
fi

# 2. The recommended '*' rewrite actually matches the real bracketed path
# (this is the wxa-jake-ai#783 fix, verified against the classifier's own
# minimatch options).
cat > "$tmp/repo/.github/risk-paths.yml" <<'YAML'
blocked: []
sensitive:
- 'src/routes/api/chat/*/stream/+server.ts'
YAML
cls=$(cd "$tmp/repo" && echo "src/routes/api/chat/[id]/stream/+server.ts" | node "$tmp/classify.mjs")
if [ "$cls" = "sensitive" ]; then
echo "✓ '*' rewrite classifies the literal [id] path as sensitive"
else
echo "✗ expected 'sensitive' for the [id] path under the '*' rewrite, got '$cls'"
failed=1
fi

# 3. A clean rules file still classifies normally (no false positive).
cls=$(cd "$tmp/repo" && echo "README.md" | node "$tmp/classify.mjs")
if [ "$cls" = "standard" ]; then
echo "✓ clean rules file classifies normally"
else
echo "✗ expected 'standard' for README.md, got '$cls'"
failed=1
fi

# 4. Intentional character classes are ALSO rejected — this is a decision,
# not an oversight. '[id]' is syntactically a valid char class, so intent
# can't be told apart; authors must enumerate ('*.js' + '*.ts'). The
# message must say so.
cat > "$tmp/repo/.github/risk-paths.yml" <<'YAML'
blocked: []
safe_test:
- 'tests/**/*.[jt]s'
YAML
set +e
out=$(cd "$tmp/repo" && echo "README.md" | node "$tmp/classify.mjs" 2>&1)
rc=$?
set -e
if [ "$rc" -ne 0 ] && echo "$out" | grep -q "enumerate"; then
echo "✓ intentional char class rejected with the enumeration hint"
else
echo "✗ expected nonzero exit + 'enumerate' in message for char-class pattern; got rc=$rc:"
echo "$out" | sed 's/^/ /'
failed=1
fi

# 5. 'always_review' is validated too: classify.mjs never matches against it,
# but codex-gate.mjs consumes it with the same minimatch semantics and is
# permissive on config errors — this guard is the only fail-closed pass
# over that list.
cat > "$tmp/repo/.github/risk-paths.yml" <<'YAML'
blocked: []
always_review:
- 'src/routes/[id]/+page.ts'
YAML
set +e
out=$(cd "$tmp/repo" && echo "README.md" | node "$tmp/classify.mjs" 2>&1)
rc=$?
set -e
if [ "$rc" -ne 0 ] && echo "$out" | grep -q "always_review"; then
echo "✓ bracket pattern under always_review fails closed"
else
echo "✗ expected nonzero exit + 'always_review' in message; got rc=$rc:"
echo "$out" | sed 's/^/ /'
failed=1
fi

exit "$failed"
1 change: 1 addition & 0 deletions selftest/test_workflow_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"script",
[
"selftest/test_automerge_risk_patterns.sh",
"selftest/test_classify_bracket_guard.sh",
"selftest/test_pr_files_listing.sh",
"selftest/test_prettier_symlink_filter.sh",
],
Expand Down