feat(enrichment): detect GitHub Actions workflow-injection and pwn-request risk#3410
feat(enrichment): detect GitHub Actions workflow-injection and pwn-request risk#3410joaovictor91123 wants to merge 1 commit into
Conversation
…quest risk Adds a REES analyzer that flags the pull_request_target/workflow_run "pwn request" pattern: checkout of an untrusted PR head, unsafe shell interpolation of untrusted event fields, and a missing permissions narrowing block on an elevated-trust trigger.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - reject/close recommendedReview updated: 2026-07-05 06:33:25 UTC
🛑 Suggested Action - Reject/Close
Review summary Blockers
Nits — 6 non-blocking
Why this is blocked
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
|
Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/workflow-injection.ts:96 treats `PULL_REQUEST_TARGET_RE` matches on comments or arbitrary string values as an active trigger, so a changed workflow containing only `# pull_request_target` plus no visible `permissions:` will emit a `missing-permissions` finding even though the workflow has no elevated trigger; change this to parse or at least key-match YAML trigger declarations, e.g. only accept `pull_request_target:` / `workflow_run:` under `on:` or the bracket/list trigger forms, or explain why comment/scalar matches are acceptable for this analyzer.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3410 +/- ##
=======================================
Coverage 93.46% 93.46%
=======================================
Files 289 289
Lines 30784 30784
Branches 11220 11220
=======================================
Hits 28773 28773
Misses 1355 1355
Partials 656 656
🚀 New features to boost your workflow:
|
Summary
Adds
review-enrichment/src/analyzers/workflow-injection.ts, a new REES analyzer that flags the GitHub Actions "pwn request" trust-boundary pattern: apull_request_target/workflow_runworkflow (which runs with the base repo's secrets and token even for a fork PR) that checks out the untrusted PR head, interpolates untrusted event fields (PR title/body/head ref, issue/comment body) directly into arun:shell step instead of viaenv:, or carries no narrowedpermissions:block. Registered inregistry.ts/analyzer-metadata.jsonalongside the existingactionPin/iacMisconfiganalyzers, whose shape it follows.No issue is linked: this repo's
linkedIssuePolicyispreferred, not required, and the proposal, rationale, and full test plan are self-contained in this PR. I checked for duplicates first: PR #2668 (workflowPermissions) proposed a related but narrower analyzer (permission-escalation flags and bare trigger-declaration detection) and was closed for a specific correctness bug in its trigger tracking; it never merged, so nothing in main covers this today. This PR does not overlap it — it targets the untrusted-checkout and shell-injection vectors #2668 didn't attempt — and its own trigger detection reads both added and unchanged context lines in a hunk (not just added lines gated behind seeingon:in the same hunk), which avoids the specific defect that got #2668 closed.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:mcp-packcrashes on my local Windows dev machine with a pre-existing, unrelatedERR_INVALID_ARG_TYPEinscripts/check-mcp-package.mjsthat I confirmed reproduces identically on a clean, unmodifiedmaincheckout — it is a local Node/Windows environment issue, not something this PR introduces. Likewisenpm run ui:lintand part ofnpm run test:coverageshow failures on this machine (CRLF-vs-LF noise across the wholeapps/gittensory-uitree fromcore.autocrlf, and ~30 shell/script-based test files that needbash/sentry-cli/dockertooling this Windows box doesn't have); I verified every one of these also fails identically on a cleanmaincheckout, so I'm listing them as run rather than skipped, with this caveat. The new analyzer's own test suite (review-enrichment/test/workflow-injection.test.ts, run vianpm run rees:test) passes in full (10/10), and the fullreview-enrichmentsuite is green apart from those same 2 pre-existingsentry-upload.test.tsfailures.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — this PR adds a new REES analyzer (backend/enrichment logic) with an accompanying
apps/gittensory-ui/src/lib/rees-analyzers.tsmetadata entry, but no new visible UI surface.Notes