Skip to content

Add GLM inline review workflow - #79

Open
frankbria wants to merge 1 commit into
mainfrom
add-glm-review
Open

Add GLM inline review workflow#79
frankbria wants to merge 1 commit into
mainfrom
add-glm-review

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Adds the reusable GLM-5.2 inline PR reviewer (frankbria/glm-review): CodeRabbit-style inline comments on defective lines with committable suggestions, bugs-only scope. ZHIPU_API_KEY secret is set. Verified on hai-sh#75.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @frankbria's task in 4m 54s —— View job


🔍 GLM Review — PR #79

Reviewed .github/workflows/glm-review.yml for concrete defects. Two findings (security/integrity), posted inline on the diff.

Severity File:line Finding
major .github/workflows/glm-review.yml:16 Reusable workflow pinned to floating @main ref — runs with PR/issue write access; any push to glm-review silently changes executed code. Violates repo's SHA-pinning policy.
minor .github/workflows/glm-review.yml:21 id-token: write grants OIDC minting to a floating-ref reviewer workflow it likely doesn't need — unnecessary escalation surface.

The if guard (changed_files >= 5 || additions >= 20 || deletions >= 20) and paths-ignore logic are correct, and permissions placement for a reusable-workflow call is valid. No logic/data-loss defects.

Note: I cannot modify .github/workflows/* (GitHub App permissions disallow it) — the suggestion blocks above are for a human to apply.

  • Gather PR context and full diff
  • Inspect changed file(s) against repo conventions
  • Hunt for concrete defects (logic, security, data-loss)
  • Post inline findings + summary

github.event.pull_request.changed_files >= 5 ||
github.event.pull_request.additions >= 20 ||
github.event.pull_request.deletions >= 20
uses: frankbria/glm-review/.github/workflows/review.yml@main

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[major] The reusable workflow is pinned to the floating @main ref, so any commit pushed to frankbria/glm-review — accidental or malicious — silently changes the code this job executes, and it runs with pull-requests: write + issues: write on this repo.

Failure scenario: a compromised token or a bad push to glm-review's main rewrites the reviewer logic; the next PR here runs attacker-controlled code that can post/modify PR comments and issues, with no SHA gate at the caller side. This repo's own stated policy is to SHA-pin actions/workflows (CLAUDE.md: "actions are SHA-pinned and Dependabot keeps the pins fresh"; security-audit.yml is blocking), so this is the one footgun it already mitigates elsewhere.

Suggested change
uses: frankbria/glm-review/.github/workflows/review.yml@main
uses: frankbria/glm-review/.github/workflows/review.yml@<40-char-commit-sha> # pin to a verified SHA, e.g. @abcd1234… # v1.0.0

Replace <40-char-commit-sha> with the full SHA of the commit you verified on hai-sh#75; Dependabot can bump it.

contents: read
pull-requests: write
issues: write
id-token: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[minor] id-token: write lets the called workflow mint OIDC tokens, which a PR reviewer has no obvious need for — it only widens the blast radius of the unpinned @main reference above.

Failure scenario: combined with the floating @main ref, a malicious reviewer revision could mint OIDC tokens and attempt to assume any cloud role this repo is trusted for (if OIDC cloud trusts are ever added). Drop it unless the glm-review workflow actually calls actions/core OIDC / requestToken.

Suggested change
id-token: write

(This deletes the id-token: write line — keep it only if the called workflow requires OIDC.)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant