Skip to content

test(ci): validate council review bot end-to-end#331

Open
sonupreetam wants to merge 2 commits into
mainfrom
test/council-review-bot
Open

test(ci): validate council review bot end-to-end#331
sonupreetam wants to merge 2 commits into
mainfrom
test/council-review-bot

Conversation

@sonupreetam

Copy link
Copy Markdown
Contributor

Summary

  • Add council review workflow files (collect, consumer, reusable) to test the bot
  • Document the council review feature in AI_TOOLING.md
  • This PR exists solely to validate the council review bot produces a clean, structured review comment

Test plan

  • Collect workflow triggers on PR open
  • Consumer workflow triggers on collect completion
  • Council review action discovers bundled Divisor agents
  • OpenCode invokes Claude on Vertex AI via WIF
  • Structured JSON review is extracted from JSONL output
  • Clean review comment is posted on the PR

Do not merge — this PR is for E2E validation only.

Made with Cursor

Add council review workflow files and document the council review
feature in AI_TOOLING.md. This PR exists solely to validate the
council review bot produces a clean, structured review comment.

Signed-off-by: sonupreetam <[email protected]>
council-review:
name: AI Council Review
if: github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success'
uses: complytime/org-infra/.github/workflows/reusable_council_review.yml@feat/council-review-production # zizmor: ignore[unpinned-uses]
if: steps.gate.outputs.should_review == 'true'
id: review
continue-on-error: true
uses: unbound-force/unbound-force/council-review-action@feat/council-review-action # zizmor: ignore[unpinned-uses]
@github-actions

Copy link
Copy Markdown

CRAP Load Analysis

No Go code changes detected in this PR. No CRAP impact.

Pull latest reusable_council_review.yml that renders inline
findings in the PR comment when the review API rejects them.

Signed-off-by: sonupreetam <[email protected]>
@github-actions

Copy link
Copy Markdown

AI Council Review

This PR introduces the three-workflow AI council review chain (ci_council_review_collect.yml, ci_council_review.yml, reusable_council_review.yml) plus a docs update. The fork-safe two-workflow pattern is architecturally sound and the gate logic is correct, but two GitHub Actions are pinned to mutable branch refs instead of commit SHAs — a supply-chain risk already flagged by OpenSSF Scorecard — and the council-review-action itself is unpinned on a feature branch, creating a reproducibility gap for a workflow that gates on security review. The test PR title matches its stated purpose (e2e validation of the council bot), and all CI checks pass.


Model: google-vertex-anthropic/claude-sonnet-4-6 | Commit: 8329ddf | Inline comments: 12

Findings

  • .github/workflows/ci_council_review.yml:41[HIGH] (Adversary) reusable_council_review.yml is referenced at @feat/council-review-production, a mutable branch ref. If the branch is force-pushed or the commit changes, this workflow silently picks up different code. Pin to a commit SHA (e.g., @<sha> # feat/council-review-production) to make the reference reproducible and auditable. OpenSSF Scorecard has already flagged this (score 8, pinned-dependencies alert).
  • .github/workflows/reusable_council_review.yml:272[HIGH] (Adversary) council-review-action is pinned to @feat/council-review-action, a mutable feature branch. This is the most sensitive step in the workflow — it runs AI review with pull-requests: write permission. A force-push to that branch would silently alter what executes with write access to PRs. Pin to a commit SHA before this workflow is used in production, and remove the # zizmor: ignore suppression once pinned.
  • .github/workflows/reusable_council_review.yml:271[MEDIUM] (SRE) The continue-on-error: true on the council review step means a complete action failure (exit 1, crash, bad JSON) will silently pass the job. The TODO comment acknowledges this is temporary, but without a tracking issue or removal criteria the suppression may persist indefinitely. Add a linked issue reference in the TODO and a concrete removal trigger (e.g., 'remove after N successful prod runs').
  • .github/workflows/reusable_council_review.yml:306[HIGH] (Adversary) REVIEW_JSON is set from steps.review.outputs.review-json and then used as a filename argument in jq ... "${REVIEW_JSON}". If the action output contains shell metacharacters or path traversal sequences, this could lead to unexpected behavior. Validate that REVIEW_JSON is a safe filesystem path (e.g., assert it matches ^[a-zA-Z0-9_./-]+$) before using it as an argument to jq and gh.
  • .github/workflows/reusable_council_review.yml:288[MEDIUM] (Adversary) The Clean up previous bot comments step pipes gh api output directly into a while read loop with gh api ... --method DELETE. If gh api --paginate returns an empty line or non-numeric comment ID, the [[ -n "${comment_id}" ]] guard prevents the DELETE, which is correct. However, the || true on the DELETE suppresses errors silently — a failed deletion will not be surfaced. Consider logging the failure with echo "::warning::Failed to delete comment ${comment_id}" before the || true.
  • .github/workflows/reusable_council_review.yml:324[MEDIUM] (Adversary) $(jq '[.inline_comments[] | {path, line, side: "RIGHT", body}]' "${REVIEW_JSON}") performs command substitution inline in a jq -n --argjson argument. If REVIEW_JSON is large or contains embedded newlines, this is fragile. Use --rawfile or --slurpfile with a temp file instead of inline command substitution to avoid potential argument-length limits and quoting issues.
  • .github/workflows/ci_council_review_collect.yml:128[MEDIUM] (Adversary) ${{ github.repository }} is interpolated directly into the run: block as a string passed to --repo. While github.repository is controlled by GitHub and unlikely to be attacker-influenced for same-repo PRs, the pattern of interpolating context values directly into shell strings is discouraged by GitHub's own hardening guide. Pass it via an env: variable (REPO: ${{ github.repository }}) and reference ${REPO} in the shell body, consistent with how other steps handle this.
  • .github/workflows/ci_council_review_collect.yml:142[LOW] (Architect) ${{ github.repository }} is interpolated directly inside the jq -n block at line 142 rather than being passed via --arg. This is inconsistent with the other four arguments which all use --arg. While github.repository is trusted, mixing interpolation styles inside a single jq -n invocation violates the DRY convention and makes the pattern harder to audit. Use --arg repo "$REPO" and reference $repo in the jq filter.
  • .github/workflows/reusable_council_review.yml:222[MEDIUM] (SRE) The Check WIF credentials step uses HAS_WIF: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER != '' }} to detect credential presence. This expression always evaluates to the string 'false' in non-secret contexts (the secret resolves to empty string on forks, giving '' != ''false). Confirm this behaves correctly across fork PRs and workflow_dispatch runs where secrets may or may not be available, and add a comment explaining the expected behavior for maintainers.
  • .github/workflows/reusable_council_review.yml:308[MEDIUM] (Tester) The Post inline review step has no test path for the case where COMMENT_COUNT is a non-integer (e.g., jq fails to parse review-json and returns null or empty). The arithmetic comparison [[ "${COMMENT_COUNT}" -gt 0 ]] will produce a bash error for non-integer values, causing the step to fail. Add a guard: if ! [[ "${COMMENT_COUNT}" =~ ^[0-9]+$ ]]; then COMMENT_COUNT=0; fi.
  • .github/workflows/reusable_council_review.yml:296[LOW] (Guard) The condition steps.review.outputs.review-mode == 'inline' on the Post inline review step and steps.review.outputs.review-mode == 'comment' on the fallback step create an implicit contract with the council-review-action's output interface. If review-mode is absent or set to an unexpected value, neither posting step will run and the review will silently produce no output. Add a third step or a fallback condition (!= 'inline' && != 'comment') to emit a workflow warning when review-mode is unrecognized.
  • docs/AI_TOOLING.md:379[LOW] (Curator) The Council Review section mentions 'Divisor agent personas from the unbound-force project' but does not link to where those personas are defined or how contributors can view or influence the review criteria. Adding a link to .opencode/agents/ or the relevant unbound-force documentation would improve contributor discoverability.

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.

2 participants