mirror of https://github.com/garrytan/gstack.git
fix: shell injection via branch names + feature-branch sampling bias
Codex code review found two issues:
P1: eval $(gstack-slug) in gstack-repo-mode executes branch names as
shell. Branch names like foo$(touch${IFS}pwned) are valid git refs and
would execute arbitrary commands. Fix: compute SLUG directly with sed
instead of eval'ing gstack-slug output.
P2: git shortlog HEAD only sees current branch history. On feature
branches that haven't merged main recently, other contributors disappear
from the sample. Fix: use git shortlog on the default branch
(origin/main) instead of HEAD.
Also improved blame lookup in collaborative triage to check both the
test file and the production code it covers.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6d2d10c125
commit
ed349027f5
|
|
@ -611,10 +611,14 @@ Use AskUserQuestion:
|
||||||
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
||||||
|
|
||||||
**If "Blame + assign GitHub issue" (collaborative only):**
|
**If "Blame + assign GitHub issue" (collaborative only):**
|
||||||
- Find who last modified the failing area:
|
- Find who likely broke it. Check BOTH the test file AND the production code it tests:
|
||||||
```bash
|
```bash
|
||||||
|
# Who last touched the failing test?
|
||||||
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
||||||
|
# Who last touched the production code the test covers? (often the actual breaker)
|
||||||
|
git log --format="%an (%ae)" -1 -- <source-file-under-test>
|
||||||
```
|
```
|
||||||
|
If these are different people, prefer the production code author — they likely introduced the regression.
|
||||||
- Create a GitHub issue assigned to that person:
|
- Create a GitHub issue assigned to that person:
|
||||||
```bash
|
```bash
|
||||||
gh issue create \
|
gh issue create \
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,8 @@
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||||
eval $("$SCRIPT_DIR/gstack-slug" 2>/dev/null) || { echo "REPO_MODE=unknown"; exit 0; }
|
# Compute SLUG directly (avoid eval of gstack-slug — branch names can contain shell metacharacters)
|
||||||
|
SLUG=$(git remote get-url origin 2>/dev/null | sed 's|.*[:/]\([^/]*/[^/]*\)\.git$|\1|;s|.*[:/]\([^/]*/[^/]*\)$|\1|' | tr '/' '-')
|
||||||
[ -z "${SLUG:-}" ] && { echo "REPO_MODE=unknown"; exit 0; }
|
[ -z "${SLUG:-}" ] && { echo "REPO_MODE=unknown"; exit 0; }
|
||||||
|
|
||||||
# Validate: only allow known values (prevent shell injection via source <(...))
|
# Validate: only allow known values (prevent shell injection via source <(...))
|
||||||
|
|
@ -39,7 +40,9 @@ if [ -f "$CACHE_FILE" ]; then
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Compute from git history (90-day window)
|
# Compute from git history (90-day window)
|
||||||
SHORTLOG=$(git shortlog -sn --since="90 days ago" --no-merges HEAD 2>/dev/null | head -20)
|
# Use default branch (not HEAD) to avoid feature-branch sampling bias
|
||||||
|
DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/||' || echo "origin/main")
|
||||||
|
SHORTLOG=$(git shortlog -sn --since="90 days ago" --no-merges "$DEFAULT_BRANCH" 2>/dev/null | head -20)
|
||||||
if [ -z "$SHORTLOG" ]; then
|
if [ -z "$SHORTLOG" ]; then
|
||||||
echo "REPO_MODE=unknown"
|
echo "REPO_MODE=unknown"
|
||||||
exit 0
|
exit 0
|
||||||
|
|
|
||||||
|
|
@ -353,10 +353,14 @@ Use AskUserQuestion:
|
||||||
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
||||||
|
|
||||||
**If "Blame + assign GitHub issue" (collaborative only):**
|
**If "Blame + assign GitHub issue" (collaborative only):**
|
||||||
- Find who last modified the failing area:
|
- Find who likely broke it. Check BOTH the test file AND the production code it tests:
|
||||||
\`\`\`bash
|
\`\`\`bash
|
||||||
|
# Who last touched the failing test?
|
||||||
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
||||||
|
# Who last touched the production code the test covers? (often the actual breaker)
|
||||||
|
git log --format="%an (%ae)" -1 -- <source-file-under-test>
|
||||||
\`\`\`
|
\`\`\`
|
||||||
|
If these are different people, prefer the production code author — they likely introduced the regression.
|
||||||
- Create a GitHub issue assigned to that person:
|
- Create a GitHub issue assigned to that person:
|
||||||
\`\`\`bash
|
\`\`\`bash
|
||||||
gh issue create \\
|
gh issue create \\
|
||||||
|
|
|
||||||
|
|
@ -621,10 +621,14 @@ Use AskUserQuestion:
|
||||||
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
- Continue with the workflow — treat the pre-existing failure as non-blocking.
|
||||||
|
|
||||||
**If "Blame + assign GitHub issue" (collaborative only):**
|
**If "Blame + assign GitHub issue" (collaborative only):**
|
||||||
- Find who last modified the failing area:
|
- Find who likely broke it. Check BOTH the test file AND the production code it tests:
|
||||||
```bash
|
```bash
|
||||||
|
# Who last touched the failing test?
|
||||||
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
git log --format="%an (%ae)" -1 -- <failing-test-file>
|
||||||
|
# Who last touched the production code the test covers? (often the actual breaker)
|
||||||
|
git log --format="%an (%ae)" -1 -- <source-file-under-test>
|
||||||
```
|
```
|
||||||
|
If these are different people, prefer the production code author — they likely introduced the regression.
|
||||||
- Create a GitHub issue assigned to that person:
|
- Create a GitHub issue assigned to that person:
|
||||||
```bash
|
```bash
|
||||||
gh issue create \
|
gh issue create \
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue