ci: complete reusable caller coverage (private-repo set)#25
Conversation
Add missing reusable workflow callers: secret-scan dependency-review pr-title-lint. Brings this repo to full coverage of the org platform's applicable reusables (secret-scan + dependency-review are the org security baseline; pr-title-lint enforces Conventional Commits on the squash subject).
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q Β· automated review]
Verdict: Approve β no blocking findings; notes below (data-flow traced).
octo-version-sync PR#25 Review Report
Reviewer: Octo-Q (automated review)
PR: #25
Head SHA: 12c99b7e4fd8230681a37c2f03e92aeff97ceb4e
Scope: 3 new files, +47/β0 lines β all GitHub Actions reusable workflow callers
1. Verification Summary
| Check | Status | Evidence |
|---|---|---|
| Reusable workflows exist | β | reusable-dependency-review.yml, reusable-pr-title-lint.yml, reusable-secret-scan.yml all present in Mininglamp-OSS/.github/.github/workflows/ |
| Caller pattern matches org convention | β | All 3 callers are byte-for-byte identical with existing callers in cc-channel-octo |
| Permissions least-privilege | β | Top-level permissions: {} on all 3; job-level scoped to minimum needed |
| Trigger events correct | β | Match reusable workflow doc comments and cc-channel-octo precedent |
@v1 pin convention |
β | Consistent with org standard |
| No code execution from PR branch | β | dependency-review and secret-scan use pull_request (base checkout); pr-title-lint uses pull_request_target but reusable only reads API metadata, checks out no code |
2. Findings
No P0/P1 issues found.
P2 β @v1 tag pinning (informational, org-wide convention)
All three callers reference @v1 (a mutable tag). SHA-pinning would provide stronger supply-chain guarantees. However, this is the established org convention (cc-channel-octo uses the same pattern), and the .github repo is org-controlled trusted infrastructure. diff-scope: pre-existing convention, not introduced by this PR. No action needed on this PR.
3. Security Analysis
pull_request_target in pr-title-lint.yml β SAFE
pull_request_target runs in the context of the base branch with access to secrets and a GITHUB_TOKEN with write permissions. This is the correct trigger for commenting on fork PRs. The reusable workflow:
- Does not check out any code (no
actions/checkout) - Reads only PR metadata (number, title, owner, repo) via
github.event.*context - Consumes the title via
env(never shell-interpolated) - Only permission:
pull-requests: write(post/update/delete comments)
No code from the PR branch is executed. No secrets are exposed.
dependency-review.yml β SAFE
Uses pull_request trigger. The reusable checks out the base branch and runs actions/dependency-review-action (GitHub first-party, SHA-pinned). Permissions: contents: read + pull-requests: write (for summary comment).
secret-scan.yml β SAFE
Triggers on both pull_request and push to main. The reusable checks out code with persist-credentials: false, downloads a SHA256-verified gitleaks binary, and scans the tree only β no project code is executed. Permission: contents: read only.
4. Data Flow Trace
| Consumed data | Source | Flows correctly? |
|---|---|---|
github.event.pull_request.number |
GitHub event payload β pr_number input (type: number) |
β |
github.event.pull_request.title |
GitHub event payload β pr_title input (type: string) β consumed via env.PR_TITLE |
β |
github.event.repository.owner.login |
GitHub event payload β repo_owner input (type: string) |
β |
github.event.repository.name |
GitHub event payload β repo_name input (type: string) |
β |
github.event.pull_request.base.sha / head.sha |
Available in pull_request event context β used by gitleaks --log-opts range |
β |
All inputs match the reusable workflow's workflow_call.inputs declarations (types and required fields).
5. Conclusion
Clean, additive-only CI change. Three standard reusable workflow callers that exactly match the established org pattern. No security concerns, no functional risks.
[Octo-Q] verdict: APPROVE β No P0/P1 findings. Only a P2 informational note on @v1 tag pinning (pre-existing org convention, not actionable on this PR).
|
ι½ιζ₯ π review β verdict: APPROVED
Byte-identical template to claw-channel-octo #23, octo-daemon-cli #33, octo-fleet #34, and octo-im #17 β diff patches hash to the same π’ Reusable contract verification carried over from sibling PRs (verified against
π’ Trigger / permission rationale identical:
π’ LGTM β ship it. Completes the five-repo platform-coverage batch (claw-channel-octo #23 + octo-daemon-cli #33 + octo-fleet #34 + octo-im #17 + octo-version-sync #25). β ι½ιζ₯ π |
yujiawei
left a comment
There was a problem hiding this comment.
Code Review β PR #25 (octo-version-sync)
Summary
This PR adds three reusable-workflow callers β secret-scan, dependency-review, and pr-title-lint β to complete the org platform's CI coverage. Two of the three are correct and well-constructed. One (dependency-review.yml) does not work on this repository and currently fails on every pull request, which is a functional defect that should be resolved before merge.
Verification
| Item | Status | Evidence |
|---|---|---|
secret-scan.yml caller wiring |
β | Triggers + permissions: contents: read match the reusable's documented usage; check passes on this PR (secret-scan / Secret Scan (gitleaks) β SUCCESS) |
pr-title-lint.yml caller wiring |
β | Inputs (pr_number/pr_title/repo_owner/repo_name) exactly match reusable-pr-title-lint.yml@v1 documented usage; job perms scoped to pull-requests: write |
dependency-review.yml caller wiring |
β | Check fails on this PR (dependency-review / Dependency Review β FAILURE). See P1 below |
permissions: {} at top level (all 3) |
β | Zero-default GITHUB_TOKEN; per-job perms granted minimally β good practice |
| Reusable refs pinned | All three pin @v1 (a mutable tag), consistent with this repo's existing reusable callers. Acceptable per repo convention; noted under Suggestions |
|
pull_request_target safety (pr-title-lint) |
β | The reusable checks out no code and consumes pr_title via env, never shell/JS interpolation β no script-injection vector. pull_request_target is justified (needed to comment on fork PRs) |
Findings
P1 β dependency-review.yml fails on every PR (private repo without Advanced Security)
.github/workflows/dependency-review.yml calls reusable-dependency-review.yml@v1, which runs actions/dependency-review-action. That action requires the Dependency Graph + GitHub Advanced Security, which is unavailable on this repository.
- Repo is private with
code_security: disabled. - The check on this PR fails in ~5s with the annotation:
"Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled along with GitHub Advanced Security."
Consequences:
- The PR's stated goal β "complete reusable caller coverage" β is not met for this caller; it adds a workflow that cannot succeed here.
- It introduces a permanently red check on every future PR, which is noise that erodes signal and can mask real failures.
- PR-time dependency scanning is already provided by the existing
osv-scanner.yml(scan-pr / osv-scanpasses on this PR), so the broken caller is also largely redundant for this repo.
This is functional and should be fixed before merge. Options (pick per platform policy):
- Drop
dependency-review.ymlfrom this repo (rely on the existingosv-scannerPR job), or - Enable Dependency Graph + Advanced Security on the repo first, then land this caller, or
- Gate the job so it no-ops where GHAS is unavailable (e.g.
continue-on-erroror a guardedif:), though a guarded-to-skip check provides no real protection and is mostly cosmetic.
Note: this does not currently block merge mechanically β main branch protection lists no required status checks (required_status_checks: null) β but shipping a known-always-failing workflow contradicts the PR's intent.
Notes / Suggestions (non-blocking)
pr-title-lintcannot be validated on this PR. Because it triggers onpull_request_target, GitHub runs the workflow definition from the base branch (main), not from this PR head. The new file therefore does not execute for PR #25 (it is correctly absent from this PR's check rollup) and will only take effect on PRs opened after merge. Expected behavior β flagging so the absence of a greenpr-title-lintcheck here is not mistaken for a problem.- Mutable
@v1ref: pinning reusables to thev1tag (resolves to135e280f...today) follows this repo's existing pattern, but a moving tag means upstream changes apply without review. Consider SHA-pinning if the platform wants reproducibility; deferring to repo convention, so non-blocking.
Security review (PR classified security-sensitive)
Manually verified the items a human reviewer would care about:
- All three callers set top-level
permissions: {}and grant only the minimal job-level scopes the reusables require β no over-privileged tokens. pull_request_targetinpr-title-lint.ymlis the one elevated-trust trigger; the called reusable checks out no untrusted code and treats the PR title as data (env var, not interpolated), so there is no command/script-injection path. Safe.secret-scanreusable downloads gitleaks as a SHA256-pinned binary and runs no project code;dependency-reviewreusable reads metadata only. No new untrusted-code-execution surface introduced by this PR.
No security objections. The blocking issue is purely the non-functional dependency-review caller.
Verdict
CHANGES_REQUESTED β resolve the dependency-review.yml caller (remove, enable GHAS, or gate) so the PR does not introduce a permanently failing check. The other two callers are good to go.
|
ι½ιζ₯ π follow-up β sibling PR alert (same defect as fleet #34)
This PR's
Cross-checked the whole 5-PR batch I approved tonight (claw #23 / daemon-cli #33 / fleet #34 / im #17 / version-sync #25): four are private + GHAS-disabled and fail; only octo-im (the lone public repo) passes. Same root cause across all four private PRs. Fix options (ops/license decision, not code):
CR pending one of A/B/C. Same recommendation on the three sibling PRs. β ι½ιζ₯ π |
Jerry-Xin
left a comment
There was a problem hiding this comment.
Scope check passed: these workflow additions are repository CI/security policy changes for octo-version-sync, so they are in scope.
π¬ Non-blocking
- π΅ Suggestion: .github/workflows/pr-title-lint.yml runs on all
pull_request_targetbranches, unlike the other new PR workflows that targetmain. If this repository may accept PRs into maintenance branches, consider addingbranches: [main]for consistency. This is not blocking.
β Highlights
- The new reusable callers use minimal top-level
permissions: {}and grant only job-level permissions needed by the called workflows. pull_request_targetis used only for PR title metadata/comment automation, with no local checkout or PR code execution in the caller.dependency-reviewandsecret-scanalign with the repositoryβs existing org-level reusable workflow pattern and security baseline direction.
No blocking correctness, security, or architecture issues found in the changed files. actionlint was not installed locally, so I reviewed the YAML and reusable workflow contracts manually.
β¦ity) dependency-review-action requires the Dependency Graph + GitHub Advanced Security, which private repos don't have β it fails permanently. Keep secret-scan + pr-title-lint (which work on private repos); rely on osv-scanner for dependency CVEs here.
|
Addressed: removed the |
|
ι½ιζ₯ π re-review (R2, head
Diffed π’ Right fix. π’ secret-scan + pr-title-lint preserved. Byte-identical to siblings, R1 reusable contract verification still holds.
Final sibling status (after this lands)
After claw lands the same drop, the batch's option-C remediation is complete. A future option B (central reusable gate) would let private callers re-add LGTM β ship it. β ι½ιζ₯ π |
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q Β· automated review]
Verdict: Approve β no blocking findings; notes below (data-flow traced).
octo-version-sync PR#25 Review Report
Reviewer: Octo-Q (automated review)
PR: #25
Head SHA: 156858cf45f9c3242899439decee2ac136de107b
Scope: 2 new workflow caller files, +33/-0 lines
1. Verification Summary
| Item | Status | Evidence |
|---|---|---|
Reusable workflows exist in .github repo |
β | reusable-pr-title-lint.yml and reusable-secret-scan.yml confirmed via API |
| Input contract match (pr-title-lint) | β | Caller passes pr_number, pr_title, repo_owner, repo_name β all required inputs declared in reusable's workflow_call.inputs |
| Input contract match (secret-scan) | β | Caller passes no inputs β reusable has only optional gitleaks-version with default 8.30.1 |
| Permissions scoping | β | Both callers set top-level permissions: {} and grant minimal job-level permissions |
| Trigger correctness | β | pull_request_target for pr-title-lint (fork-safe commenting); pull_request + push for secret-scan (matches reusable's documented usage) |
| Ref pinning convention | @v1 tag β consistent with existing docker-lint.yml in same repo |
|
| dependency-review removal | β | Commit 2 correctly drops it; private repos lack GHAS. osv-scanner.yml already covers dependency CVEs |
2. Findings
P2 β @v1 moving-tag reference (supply chain)
Files: .github/workflows/pr-title-lint.yml:11, .github/workflows/secret-scan.yml:11
Both callers reference Mininglamp-OSS/.github/.github/workflows/reusable-*.yml@v1. A v1 tag can be moved to point to a different commit. If the .github repo were compromised, an attacker could redirect the reusable to malicious code.
Diff-scope: Pre-existing pattern β docker-lint.yml in the same repo already uses @v1 for its reusable caller. The new files follow the established convention. Not new or amplified by this PR.
Mitigation: The .github repo is org-controlled (Mininglamp-OSS), and GitHub's reusable workflow model does not expose GITHUB_TOKEN to the called workflow by default. Risk is low but non-zero. A future org-wide migration to SHA-pinned refs would close this.
Nit β No concurrency control on pr-title-lint
The reusable handles idempotency internally (finds/updates existing comment by <!-- octo-pr-title-lint --> marker), so rapid successive edited events won't produce duplicate comments. No action needed.
3. Data Flow Trace
pr-title-lint.yml
| Consumer | Source | Flow verified |
|---|---|---|
pr_number input |
github.event.pull_request.number |
β
Integer, always present on pull_request_target |
pr_title input |
github.event.pull_request.title |
β
String, consumed via process.env.PR_TITLE in reusable (never shell-interpolated). Backtick-escaped in comment body (.replace(//g, "'")`) |
repo_owner input |
github.event.repository.owner.login |
β String, always present |
repo_name input |
github.event.repository.name |
β String, always present |
pull-requests: write permission |
Used by reusable to post/update/delete PR comments | β Scoped to job level |
Security note on pull_request_target: The reusable does NOT checkout PR code β it only reads PR metadata via the GitHub API. This is the correct fork-safe pattern for PR validation workflows that need write access.
secret-scan.yml
| Consumer | Source | Flow verified |
|---|---|---|
contents: read permission |
Used by actions/checkout in reusable |
β Scoped to job level |
github.event_name |
Determines scan mode (PR range vs full history) | β Handled correctly in reusable |
BASE_SHA / HEAD_SHA |
PR commit range for gitleaks | β
Only used on pull_request events |
Reusable security posture: gitleaks binary is SHA256-verified on download; persist-credentials: false on checkout; no project code is executed.
4. Additional Observations
- dependency-review rationale is sound. The commit 2 message correctly identifies that
dependency-review-actionrequires Dependency Graph + GitHub Advanced Security (unavailable on private repos). The existingosv-scanner.ymlprovides equivalent CVE coverage. - Both reusables are well-engineered. The pr-title-lint reusable uses
github-scriptwith env-based input (no string interpolation of untrusted data). The secret-scan reusable uses pinned, checksum-verified binary downloads.
5. Verdict
No P0 or P1 issues found. The single P2 (moving-tag ref) follows the repo's established convention and is mitigated by org-level control of the .github repo. All input contracts match, permissions are correctly scoped, and the pull_request_target usage is safe (no code checkout from PR branch).
[Octo-Q] verdict: APPROVE β No blocking issues. The P2 supply-chain note is informational and consistent with existing repo patterns.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review β PR #25 (octo-version-sync)
Summary
This PR completes the org reusable-workflow coverage for this repo by adding two CI callers: secret-scan and pr-title-lint. The previously-flagged dependency-review caller (which failed permanently on this private, non-GHAS repo) has been removed at this head, resolving the blocker from the prior review. Both remaining callers are correctly wired, least-privileged, and security-clean. Approving.
Verification
| Item | Status | Evidence |
|---|---|---|
Prior blocker (dependency-review.yml) resolved |
β | File no longer present in the diff at head 156858cf; PR now adds only secret-scan.yml (+15) and pr-title-lint.yml (+18), +33/-0 |
secret-scan.yml caller wiring |
β | pull_request/push to main triggers + job-level contents: read match reusable-secret-scan.yml@v1 documented usage; check secret-scan / Secret Scan (gitleaks) β SUCCESS on this PR |
pr-title-lint.yml caller wiring |
β | All four required inputs (pr_number, pr_title, repo_owner, repo_name) wired from github.event.* exactly per reusable-pr-title-lint.yml@v1; job perms scoped to pull-requests: write |
Top-level permissions: {} (both files) |
β | Zero-default GITHUB_TOKEN; per-job scopes granted minimally β good least-privilege practice |
actionlint / workflow sanity |
β | sanity / actionlint and sanity / No tabs in workflow files both SUCCESS |
| Reusable refs pinned | Both pin mutable @v1, consistent with this repo's existing caller convention and an org-controlled .github repo. Acceptable; noted under Suggestions |
Security review (PR classified security-sensitive)
Reviewed against the actual contents of the called reusables in Mininglamp-OSS/.github:
pull_request_targetinpr-title-lint.ymlβ SAFE. This is the one elevated-trust trigger (needed so the workflow can comment on fork PRs). The called reusable (reusable-pr-title-lint.yml@v1) checks out no code, reads PR metadata via the API only, and consumes the PR title via anenvvariable (PR_TITLE) that is never shell- or string-interpolated into thegithub-scriptbody β so there is no script/command-injection path from an attacker-controlled title. It usesactions/github-scriptSHA-pinned to3a2844b7β¦(v9.0.0) and runs with onlypull-requests: write. No untrusted-code execution surface.secret-scan.ymlβ SAFE. Usespull_request(base-context) +pushtriggers; the reusable checks out withpersist-credentials: false, downloads gitleaks as a SHA256-pinned binary (551f6fc8β¦, v8.30.1, version-mismatch guarded), checkout pinned tode0fac2eβ¦(v6.0.2), and scans the tree/commit-range only β it runs no project code. Permission:contents: read.- No secrets exposed and no new untrusted-code-execution surface is introduced by either caller.
Notes / Suggestions (non-blocking)
pr-title-lintdoes not produce a green check on this PR β expected. Because it triggers onpull_request_target, GitHub runs the workflow definition from the base branch (main), where the file does not yet exist. It will take effect on PRs opened after this merges. The absence of apr-title-lintcheck on this PR is not a problem.- Branch-filter consistency (nit).
secret-scan.ymlfiltersbranches: [main], whilepr-title-lint.ymlruns onpull_request_targetfor all base branches. If this repo only takes PRs intomain, addingbranches: [main]topr-title-lintwould make the two consistent. Purely cosmetic. synchronizeinpr-title-linttypes (nit). The title cannot change on asynchronizeevent, so that type is effectively a harmless no-op re-run. Fine to leave as-is for parity with the org template.- Mutable
@v1ref. Both callers pin the movingv1tag (org convention). SHA-pinning would give stronger supply-chain reproducibility, but the.githubrepo is org-controlled trusted infrastructure; deferring to repo convention β non-blocking.
Unrelated CI note (not a review blocker)
The check-sprint / check-sprint check is red on this PR, but it fails at the "Validate Sprint field on Project Board" step β a pre-existing org project-board governance workflow unrelated to this PR's diff. Not introduced by this change and not a code-quality concern for these two files. (main branch protection lists no required status checks, so it does not mechanically block merge either.)
Verdict
APPROVED β the prior blocker is resolved; the two remaining callers are correctly wired, least-privileged, and security-clean. Remaining items are non-blocking nits.
Jerry-Xin
left a comment
There was a problem hiding this comment.
This PR is project-relevant, but it does not implement one of its stated security-baseline callers.
π΄ Blocking
- π΄ Critical β Missing dependency-review workflow. The PR description says this adds
secret-scan,dependency-review, andpr-title-lint, completing reusable caller coverage, but the diff only adds.github/workflows/pr-title-lint.ymland.github/workflows/secret-scan.yml. There is no.github/workflows/dependency-review.ymlor any existing dependency-review caller in the repository. This leaves the advertised security baseline incomplete and could cause maintainers to believe dependency review is enforced when it is not.
π¬ Non-blocking
- π΅ Suggestion β
.github/workflows/pr-title-lint.yml:4usespull_request_target, which is reasonable for metadata-only PR automation, but the repoβs similar workflows include comments explaining why this is safe. Consider adding the same security note and, if applicable, the existingzizmorignore style used in.github/workflows/labeler.yml.
β Highlights
.github/workflows/secret-scan.yml:9-15keeps permissions minimal and only grantscontents: read..github/workflows/pr-title-lint.yml:7-18avoids inheriting broad top-level permissions and grants onlypull-requests: writeto the reusable caller.
|
@Jerry-Xin good catch on the description mismatch β I've corrected it. dependency-review is intentionally omitted on this private repo: dependency-review-action needs Dependency Graph + GitHub Advanced Security (unavailable on private repos), so it would be a permanent red check. osv-scanner already covers dependency CVEs here. The PR now correctly advertises only secret-scan + pr-title-lint. Please re-review. |
Add the security rationale comment (metadata-only; no PR-code checkout; title read via env, not interpolated) matching the repo's other pull_request_target workflows. Addresses review feedback.
β
Review β PR #25 (
|
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q Β· automated review]
Verdict: Approve β no blocking findings; notes below (data-flow traced).
octo-version-sync PR#25 Review Report
Reviewer: Octo-Q (automated review)
PR: #25
Head SHA: e831037e67f0c65a7feba6f2da48f72f950e8e41
Routing: complexity=security_sensitive (automated review)
1. Verification Summary
| Item | Status | Evidence |
|---|---|---|
pr-title-lint.yml β pull_request_target safety |
β | No PR code checkout; metadata-only; title via env; matches existing labeler.yml pattern |
pr-title-lint.yml β permissions minimization |
β | Workflow-level permissions: {}; job-level pull-requests: write (needed for comment API) |
pr-title-lint.yml β title injection risk |
β | Title consumed via process.env.PR_TITLE; backtick-sanitized (.replace(//g, "'")`) before markdown comment body; regex types from hardcoded default |
secret-scan.yml β trigger safety |
β | Uses pull_request (not _target); permissions: {} at workflow level; job contents: read only |
secret-scan.yml β supply-chain |
β | Reusable downloads gitleaks with SHA256 pin; actions/checkout SHA-pinned with persist-credentials: false |
| Reusable workflow pinning | β | Both reference @v1 tag (consistent with all other callers in this repo) |
| Parity with existing workflows | β | pull_request_target + permissions: {} + safety comment + zizmor: ignore matches labeler.yml, octo-pr-result-notify.yml, octo-pr-review-feed.yml |
2. Findings
No P0/P1/P2 findings.
Both workflow caller files are well-structured and follow the established security patterns already present in this repository.
Detailed analysis
pr-title-lint.yml (21 lines, new file)
- Uses
pull_request_targetβ the dangerous trigger β but correctly mitigates all known attack vectors:- No code checkout: The reusable workflow (
reusable-pr-title-lint.yml@v1) uses onlyactions/github-script(SHA-pinned3a2844b7). It never callsactions/checkoutand never executes PR branch code. - Title via env, not interpolation:
pr_titleinput βenv.PR_TITLEβprocess.env.PR_TITLEin JS. Never shell-interpolated. - Markdown injection mitigated: Title appears in a comment body inside backtick code span, with
.replace(//g, "'")` preventing escape. core.setFailed()logging: Title in log output is informational only, no execution risk.
- No code checkout: The reusable workflow (
- Safety rationale comment (commit 3) documents the
pull_request_targetdecision withzizmor: ignore[dangerous-triggers]β matches the pattern inlabeler.ymlline-for-line. - Trigger types
[opened, edited, reopened, synchronize]are correct for title validation (must re-run on title edit).
secret-scan.yml (15 lines, new file)
- Uses safe
pull_request+pushtriggers onmainonly. - Delegates entirely to
reusable-secret-scan.yml@v1which: downloads gitleaks with SHA256 verification, usesactions/checkoutwithpersist-credentials: false, scans git history only (no project code execution). permissions: {}at workflow level; job-levelcontents: readβ minimal.
3. Suggestions
None blocking. The PR is clean and ready to land.
4. Additional Observations
- The PR body correctly explains why
dependency-reviewwas intentionally excluded (private repos lack GitHub Advanced Security). This is consistent withclaw-channel-octo,octo-daemon-cli,octo-fleet. - The 3-commit history tells a clear story: add all three β drop dependency-review β add safety rationale. Clean progression.
5. Data Flow Trace
pr-title-lint.yml:
github.event.pull_request.titleβ inputpr_titleβ reusable input βenv.PR_TITLEβprocess.env.PR_TITLEβ regex test + sanitized comment body + log. No interpolation, no code execution. Safe.github.event.pull_request.numberβ inputpr_numberβ reusable input βenv.PR_NUMBERβparseInt()β API callissue_number. Safe.github.event.repository.owner.login/.nameβ inputs βenvβ API calls. Safe.
secret-scan.yml:
- No inputs passed (uses defaults). Reusable internally reads
github.event_name,github.event.pull_request.base.sha,github.event.pull_request.head.shavia env for scan range. Safe β standard GitHub event context, no user-controlled interpolation.
6. R5 Blind-Spot Checklist (security_sensitive)
- C1 β Dual-path parity: N/A. No symmetric add/remove or create/delete paths. Single-direction CI workflows.
- C2 β Control-flow ordering / nested reuse: Clear. Each caller invokes exactly one reusable; no nested/chained calls. Title sanitization (
.replace(//g, "'")`) tested against backtick injection β effective for the markdown code-span context. - C3 β Authorization boundary β capability boundary: Clear.
pr-title-lint.ymlusespull_request_target(grants write token to fork PRs), but the reusable only performs metadata API reads and comment writes β no code checkout, no secret access, no arbitrary execution. The capability exposed (post/delete/update comments on the PR) matches the authorization intent.secret-scan.ymlusespull_request(no write token to forks).
[Octo-Q] verdict: APPROVE β No P0/P1/P2 findings. Both workflow callers follow the established security patterns in this repo. The pull_request_target usage in pr-title-lint.yml is correctly mitigated (metadata-only, no code checkout, title via env). secret-scan.yml uses safe triggers with minimal permissions.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review β PR #25 (octo-version-sync)
Summary
This PR adds the two reusable-caller workflows applicable to this private repo β secret-scan.yml and pr-title-lint.yml β completing the org's standard CI coverage. Net diff is 36 additions across 2 new files, no deletions. The change is purely additive CI configuration.
Verdict: APPROVED. No P0/P1 issues. The implementation matches the established private-repo pattern exactly, and the security-relevant callees were independently verified to be safe.
1. Verification
- β
secret-scan.ymlis byte-identical to the established private-repo pattern. It matchesocto-fleet/.github/workflows/secret-scan.yml(a private sibling) line for line:pull_request+pushonmain, top-levelpermissions: {}, job-levelcontents: read, callingreusable-secret-scan.yml@v1. - β
pr-title-lint.ymlis byte-identical to the established pattern. It matchesocto-fleet/.github/workflows/pr-title-lint.yml:pull_request_target(types opened/edited/reopened/synchronize), top-levelpermissions: {}, job-levelpull-requests: write, callingreusable-pr-title-lint.yml@v1. - β
No duplication. Neither
secret-scan.ymlnorpr-title-lint.ymlpreviously existed in this repo; this genuinely adds new coverage rather than colliding with existing workflows. - β
pull_request_targetis used safely (the main fork-PR risk vector). The reusablereusable-pr-title-lint.yml@v1checks out NO PR code β it reads PR metadata via the API only. The PR title is passed as aworkflow_callinput, exposed viaenv: PR_TITLE, and consumed in github-script viaprocess.env.PR_TITLE. It is never shell- or JS-string-interpolated into a script body, so a malicious fork PR title cannot inject code or commands. The inline comment in the caller and the org-standardzizmor: ignore[dangerous-triggers]annotation correctly document this. - β
secret-scancallee is supply-chain hardened.reusable-secret-scan.yml@v1checks out withpersist-credentials: false, pinsactions/checkoutto a full SHA, downloads gitleaks as a pinned version with SHA256 verification (sha256sum -c), and runs only gitleaks against the tree/history β no project code is executed. - β
Least privilege. Both workflows declare
permissions: {}at the top level and grant only the minimal scope each job needs (contents: read/pull-requests: write). - β
dependency-review omission is correctly justified. This repo already ships
osv-scanner.yml, which covers dependency CVEs.dependency-review-actionrequires GitHub Advanced Security, which is unavailable on private repos, so it would fail permanently. Excluding it is the right call and consistent with the other private repos (octo-fleet,octo-daemon-cli).
2. Findings
P2 / nit β minor inaccuracy in the PR description (non-blocking)
The PR body states the dependency-review omission "matches the other private repos: claw-channel-octo, octo-daemon-cli, octo-fleet." The repo is actually named openclaw-channel-octo, and it is public (not private) β it ships its own dependency-review.yml precisely because public repos get Advanced Security for free. So it is not a valid example of the "private repos skip dependency-review" rationale. The two genuinely-private examples (octo-daemon-cli, octo-fleet) are correct and sufficient. This affects only the prose justification, not the code. No change required to merge.
3. Suggestions
- (Optional, description-only) Drop or correct the
claw-channel-octoreference in the PR body so the "private repos" rationale cites only actually-private repos.
4. Additional observations (for the human reviewer β security_sensitive PR)
- Mutable
@v1tag pinning. Both callers reference the reusable workflows by the mutable tag@v1rather than a commit SHA. If@v1were force-moved inMininglamp-OSS/.github, behavior here would change without a PR. This is the established org-wide convention β all 16 existing workflows in this repo pin reusables by@v1β so this PR is consistent and I am not flagging it as a blocker. I surface it only because the classification asks for items worth a human's manual awareness: the trust boundary for both new workflows is the.githuborg repo'sv1tag, which is the same trust boundary the rest of this repo's CI already depends on. - No secrets, tokens, or credentials are introduced by this diff. The
secret-scanworkflow's purpose is, in fact, to catch them.
Quick fact-check on the "private repo names" nitRe yujiawei's π‘ that the PR body cites wrong private-repo example names β I checked the actual visibility via the API, and the PR description is correct as written:
All three repos the PR names as "the other private repos" are genuinely private, so the parity claim and the example names are accurate β no correction needed there. (For reference, So I'd treat that π‘ as a non-issue; the body doesn't need editing. The substantive verification stands: callers are correct, |
π Review-state note β one stale CHANGES_REQUESTED on an old SHAHeads up before merge: the latest review per reviewer is β
Both approvals are on the current HEAD. The one open π΄ is Jerry-Xin's, still pinned to the older That earlier RC was the So this PR is correct as-is; the blocking π΄ is stale and was a false positive. Under typical branch protection an unresolved CHANGES_REQUESTED still blocks merge regardless of the "2 approvals" signal β so it likely needs @Jerry-Xin to re-review on |
Jerry-Xin
left a comment
There was a problem hiding this comment.
Adds relevant repository maintenance workflows for PR title linting and secret scanning; the PR is in scope for octo-version-sync.
π¬ Non-blocking
No issues found.
β
Highlights
π΅ Suggestion: The pull_request_target usage in .github/workflows/pr-title-lint.yml:3 is appropriately documented as metadata-only, and the caller passes only PR metadata into the reusable workflow.
π΅ Suggestion: .github/workflows/secret-scan.yml:9 keeps default permissions empty and grants only contents: read at .github/workflows/secret-scan.yml:14, matching least-privilege expectations for checkout-based scanning.
Validation performed: YAML parse check passed for both new workflow files, and git diff --check reported no whitespace issues.
Adds the reusable callers applicable to this private repo:
secret-scan+pr-title-lint.Note on dependency-review: intentionally NOT included.
dependency-review-actionrequires the Dependency Graph plus GitHub Advanced Security, which private repos don't have β it fails permanently. Dependency CVEs are already covered here byosv-scanner. (This matches the other private repos: claw-channel-octo, octo-daemon-cli, octo-fleet.)