Skip to content

feat(enrichment): flag risky workflow permissions and triggers#2668

Closed
jaso0n0818 wants to merge 2 commits into
JSONbored:mainfrom
jaso0n0818:feat/workflow-permissions-analyzer
Closed

feat(enrichment): flag risky workflow permissions and triggers#2668
jaso0n0818 wants to merge 2 commits into
JSONbored:mainfrom
jaso0n0818:feat/workflow-permissions-analyzer

Conversation

@jaso0n0818

Copy link
Copy Markdown
Contributor

What

A new local REES analyzer, workflowPermissions, that flags changes to .github/workflows/* which grant broad or sensitive permissions, or add high-risk event triggers — the pull_request_target + write-token supply-chain class, where untrusted fork code runs with the base repo's secrets and a write token.

Detections (the fixed GitHub Actions schema — a bounded enum)

  • permissions: write-all
  • id-token: write (OIDC cloud-credential exchange)
  • secrets: inherit (forwards all of the caller's secrets to a reusable workflow)
  • sensitive per-scope writes — contents / packages / actions / deployments / security-events : write (reports the scope)
  • pull_request_target and workflow_run triggers, matched only as a trigger declaration — an on: line naming it, a mapping key, or a - list item — so the token merely appearing in a name/value is not flagged

Why it is bounded / merge-safe

It matches the fixed GitHub Actions workflow schema (a documented enum of permission scopes and event names), never arbitrary code, so it has no unbounded parsing edge cases. YAML # comments are stripped before matching. It mirrors the existing actions-pin and iac-misconfig line-cited patch analyzers (per-line scan, hunk tracking, MAX_FINDINGS/MAX_LINE_CHARS caps, dedup by kind+line, fail-safe on abort).

Value

No existing analyzer covers workflow permission escalation or dangerous event triggers. A PR that adds pull_request_target (running untrusted fork code with the base repo's secrets) or a broad write-all grant is a well-known supply-chain vector worth surfacing before merge.

Tests

review-enrichment/test/workflow-permissions.test.ts covers every kind, each sensitive scope, YAML-comment stripping, read-grant and actions/* uses-ref non-matches, new-file line tracking across removed/context lines, per-kind dedup, the budget/abort paths, and the workflow-path filter. Analyzer metadata is regenerated and committed.

No linked issue

No linked issue because this is a net-new analyzer; there is no tracking issue to link.

@jaso0n0818
jaso0n0818 requested a review from JSONbored as a code owner July 3, 2026 02:46
@dosubot dosubot Bot added the size:L label Jul 3, 2026
@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-03 05:49:26 UTC

10 files · 1 AI reviewer · 1 blocker · readiness 80/100 · CI green · blocked

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/workflow-permissions.ts:129 resets `OnBlockTracker` at every hunk and only enters the block after seeing `on:`, so adding `pull_request_target:` or `- workflow_run` under an existing `on:` block whose `on:` line is outside the diff context produces no finding even though the PR claims those trigger declarations are detected. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The PR adds a bounded local analyzer for risky GitHub Actions permissions/triggers and wires it through registry metadata, scheduling, rendering, and tests. The permission-line checks are straightforward, but the trigger tracking is incomplete for a real existing-workflow edit: block/list trigger declarations are only detected when the hunk also contains the top-level `on:` line, so common additions under an existing `on:` block are missed.

Blockers

  • review-enrichment/src/analyzers/workflow-permissions.ts:129 resets `OnBlockTracker` at every hunk and only enters the block after seeing `on:`, so adding `pull_request_target:` or `- workflow_run` under an existing `on:` block whose `on:` line is outside the diff context produces no finding even though the PR claims those trigger declarations are detected.
Nits — 4 non-blocking
  • nit: review-enrichment/src/analyzers/workflow-permissions.ts:78 `stripYamlComment` treats `#` after any whitespace as a comment, which can false-negative quoted YAML values containing space-plus-hash; acceptable for this bounded scanner, but worth documenting as a deliberate tradeoff if kept.
  • nit: review-enrichment/test/workflow-permissions.test.ts:119 covers the hunk-gap non-carry behavior only as a non-match; add the real positive case for inserting a trigger into an existing `on:` block once the analyzer can infer or otherwise handle that context.
  • In `review-enrichment/src/analyzers/workflow-permissions.ts`, either parse enough surrounding workflow content to know whether the added direct child is under top-level `on:`, or make the trigger matcher conservative-but-complete for added child declarations using available hunk context and document the residual false-positive/false-negative boundary.
  • In `review-enrichment/test/workflow-permissions.test.ts`, add a patch where `@​@​` context contains only existing sibling trigger lines and the added line is `+ pull_request_target:`; that is the common edit path this analyzer needs to catch.

Why this is blocked

  • review-enrichment/src/analyzers/workflow-permissions.ts:129 resets `OnBlockTracker` at every hunk and only enters the block after seeing `on:`, so adding `pull_request_target:` or `- workflow_run` under an existing `on:` block whose `on:` line is outside the diff context produces no finding even though the PR claims those trigger declarations are detected.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:L; no linked issue context).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 424 registered-repo PR(s), 272 merged, 6 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jaso0n0818; Gittensor profile; 424 PR(s), 6 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: jaso0n0818
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 424 PR(s), 6 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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.

  • Re-run Gittensory review

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing, see gittensory review for blockers.

AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/workflow-permissions.ts:129 resets `OnBlockTracker` at every hunk and only enters the block after seeing `on:`, so adding `pull_request_target:` or `- workflow_run` under an existing `on:` block whose `on:` line is outside the diff context produces no finding even though the PR claims those trigger declarations are detected. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Development

Successfully merging this pull request may close these issues.

2 participants