Repository navigation
ci(guardian): a reusable review workflow other repositories call - #740
Conversation
The review job moves to .github/workflows/guardian-reusable.yml (`on: workflow_call`). guardian.yml becomes its first caller, at the commit the event ran on; its save-metrics and rate jobs are unchanged. The workflow takes these inputs: pr_number, tooling_ref (the codegraph-brain commit whose tooling runs), source_root, runner (JSON runs-on) and save_metrics. Secrets and vars are the caller's, so an organisation can set them once. The shape from #731 and #734 is kept: - tooling is codegraph-brain at tooling_ref, and the PR is data in pr/; - only one fixed git fetch runs there, now authenticated with the caller's GITHUB_TOKEN for this one command, so private repositories work; - the app token is scoped to the caller's repository and to pull-requests: write. The write-access check lives in the reusable job, so no caller can drop it. guardian_review.py gains --source-root, which refuses an absolute path or `..`. The workflow tests now pin the reusable file and the caller. Closes #739 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --source-root argument to the Guardian review script, allowing repositories to specify a custom source directory inside the checkout. It adds validation to ensure the source root is a relative path within the checkout and updates the associated GitHub Actions workflows and unit tests. The review feedback highlights a potential path traversal vulnerability on Windows systems where backslashes are used as path separators. Since PurePosixPath does not recognize backslashes, a path like ..\\elsewhere could bypass validation. The reviewer suggests using both PurePosixPath and PureWindowsPath for robust validation, importing PureWindowsPath, and expanding the test suite to cover Windows-style and absolute paths.
…ten the job Review of #740: - `secrets: inherit` stops at the organisation, so an Ownima caller of this user-owned workflow got no keys and could not pass any. The four secrets are now declared under workflow_call (optional), and the header shows the explicit form, the Actions policy the caller needs, and the same-SHA pin for uses: and tooling_ref. - A step fails fast when tooling_ref predates the workflow (no --source-root in the script), instead of on an unknown flag. - The base fetch takes the token from git's environment config, not argv, scoped to github.com and masked as actions/checkout masks it. - The job refuses an event other than issue_comment with a plain message. - save_metrics says it reads codegraph-brain's own branch. - Tests: the pr/ step's commands and env are pinned exactly (the old no-secret check no longer looked at github.token); the validation step's own shell runs for five cases; the restore's `if`, the declared secrets, the app token's owner, the caller's whole `if` and an absolute --source-root are covered. Each new pin was seen failing on its mutation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From Gemini's review of #740: a backslash has no use in a repository path and would hide a `..` on a platform that treats it as a separator. Refused in guardian_review.py and in the workflow's own check, with tests for `..\\x`, `C:\\x` and `a/../..`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/gemini review |
|
/guardian review |
![codegraph-brain[bot]](https://avatars.githubusercontent.com/in/4450645?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: Checked the source_root refusal logic in relative_source_root and its bash twin: '..', absolute paths, and backslashes are refused before any spend, and the tests cover those cases. Checked the PR-checkout isolation: only the base fetch runs in pr/, it carries only the caller's GITHUB_TOKEN via git's environment config, and no provider key reaches it. Checked the caller/reusable split: the secrets declarations, the job-level if, and the permissions match the reusable job's needs.
🤖 claude-haiku-5-5 · 24,807 prompt + 10,693 completion = 35,500 tokens · graph 1/6 files (17%)
There was a problem hiding this comment.
Code Review
This pull request introduces support for a configurable --source-root directory in the Guardian review script, allowing repositories with alternative source layouts to be analyzed. It includes path validation to prevent directory traversal outside the checkout, updates the GitHub Actions workflow to a reusable format, and adds comprehensive unit tests. The review feedback suggests enhancing the path validation by utilizing PureWindowsPath to robustly detect Windows-style absolute paths across all platforms, along with adding corresponding test cases.
From the second review round of #740 (guardian and Gemini): - The workflow's shell check and relative_source_root now apply the same rule. `..` counts only as a path component (`my..pkg` is fine); an absolute path, a drive (C:/x), a backslash or any whitespace is refused. Whitespace is refused, not stripped, because the ingest gets the raw value. An empty value (the whole repository) is accepted by both. - The credential is encoded with `base64 | tr -d '\n'` rather than the GNU-only `-w0`, since the runner input allows self-hosted machines. - Tests: normalisation of '', '.', './src', 'src/'; refusals of whitespace and drives; the real ContextCollector built with source_root; the shell check for my..pkg, a/.., ' src', '' and ./src. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/gemini review |
|
/guardian review |
There was a problem hiding this comment.
Summary: Checked the shell validation of source_root against the Python relative_source_root rules (traversal, absolute, drive, backslash, whitespace, empty) and found them consistent with the added tests. Checked the PR fetch credential handling (base64 token masked, passed via GIT_CONFIG env, persist-credentials false) and the ingest path quoting, and found no defect there. Checked the caller's secrets/permissions passthrough and the timeout against the token lifetime, and found them consistent.
🤖 claude-haiku-5-5 · 25,669 prompt + 7,655 completion = 33,324 tokens · graph 1/6 files (17%)
There was a problem hiding this comment.
Code Review
This pull request introduces support for a configurable --source-root directory in the Guardian review script and GitHub workflows, allowing repositories with different source layouts to use the tool. It adds input validation to ensure the source root is a safe, relative path inside the checkout, and transitions the GitHub Action to a reusable workflow structure. Additionally, comprehensive unit tests are added to verify the new validation logic and workflow configurations. The review feedback highlights a potential runtime TypeError in the path validation function if the input is None, suggesting normalizing the input to a string before performing checks.
From guardian's third review of #740: "only the two posting steps" read as excluding the review step, which is the one that posts the inline review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Closes #739.
Change
guardian-reusable.yml(on: workflow_call) holds the review job. Inputs:pr_number;tooling_ref, the codegraph-brain commit whose tooling runs;source_root, defaultsrc;runner, a JSON runs-on;save_metrics.The caller passes secrets and vars, so they can be set once per organisation.
guardian.ymlis now its first caller, atgithub.sha, withsource_root: srcandsave_metrics: true. Its save-metrics and rate jobs are unchanged.tooling_ref, and the PR is data inpr/;git fetch, authenticated for that one command with the caller's GITHUB_TOKEN, so private repositories work;pull-requests: write.The write-access check lives in the reusable job, so no caller can drop it.
guardian_review.py --source-root: another repository's sources need not be undersrc/. An absolute path or..is refused before any spend.uv sync --directory pr, tooling from the caller's repository,fetch && uv sync.For a caller in another repository
Pin
uses: zaebee/codegraph-brain/.github/workflows/guardian-reusable.yml@<sha>and pass the sametooling_ref, using a commit SHA, not a tag. A tag can move, and the tooling runs on the caller's runner. With a SHA pin, every update reaches that repository only through a PR there that changes the SHA, and that includes a dependency bump here.🤖 Generated with Claude Code