Skip to content

ci: complete reusable caller coverage (private-repo set)#25

Merged
lml2468 merged 3 commits into
mainfrom
ci/complete-caller-coverage
Jun 10, 2026
Merged

ci: complete reusable caller coverage (private-repo set)#25
lml2468 merged 3 commits into
mainfrom
ci/complete-caller-coverage

Conversation

@lml2468

@lml2468 lml2468 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Adds the reusable callers applicable to this private repo: secret-scan + pr-title-lint.

Note on dependency-review: intentionally NOT included. dependency-review-action requires the Dependency Graph plus GitHub Advanced Security, which private repos don't have β€” it fails permanently. Dependency CVEs are already covered here by osv-scanner. (This matches the other private repos: claw-channel-octo, octo-daemon-cli, octo-fleet.)

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).
@lml2468 lml2468 requested a review from a team as a code owner June 9, 2026 12:24
@github-actions github-actions Bot added the size/S PR size: S label Jun 9, 2026
mochashanyao
mochashanyao previously approved these changes Jun 9, 2026

@mochashanyao mochashanyao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[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).

@lml2468

lml2468 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

齐静ζ˜₯ πŸ“– review β€” verdict: APPROVED

Posted via lml2468 token (self APPROVE on own-author PR is blocked β€” verdict in body).

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 3879924556... across all five PRs. Three new reusable callers: dependency-review.yml / pr-title-lint.yml / secret-scan.yml, +47/-0.

🟒 Reusable contract verification carried over from sibling PRs (verified against Mininglamp-OSS/.github):

  • reusable-pr-title-lint@v1 β€” 4 required inputs (pr_number, pr_title, repo_owner, repo_name) all wired from github.event.*
  • reusable-dependency-review@v1 + reusable-secret-scan@v1 β€” only required: false inputs, no with: needed

🟒 Trigger / permission rationale identical:

  • dependency-review β†’ pull_request (lockfile diff, no fork-code write tokens needed)
  • pr-title-lint β†’ pull_request_target (reusable doesn't checkout PR code, only env: PR_TITLE)
  • secret-scan β†’ pull_request + push branches:[main] dual trigger
  • All workflow-level permissions: {} + job-scoped minimal writes

🟒 octo-version-sync is a product-line repo, so per workflow-architecture.md §232-233 it should receive the full reusable platform. This PR closes the gap.

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 yujiawei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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:

  1. The PR's stated goal β€” "complete reusable caller coverage" β€” is not met for this caller; it adds a workflow that cannot succeed here.
  2. It introduces a permanently red check on every future PR, which is noise that erodes signal and can mask real failures.
  3. PR-time dependency scanning is already provided by the existing osv-scanner.yml (scan-pr / osv-scan passes 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.yml from this repo (rely on the existing osv-scanner PR 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-error or a guarded if:), 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-lint cannot be validated on this PR. Because it triggers on pull_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 green pr-title-lint check here is not mistaken for a problem.
  • Mutable @v1 ref: pinning reusables to the v1 tag (resolves to 135e280f... 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_target in pr-title-lint.yml is 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-scan reusable downloads gitleaks as a SHA256-pinned binary and runs no project code; dependency-review reusable 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.

@lml2468

lml2468 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

齐静ζ˜₯ πŸ“– follow-up β€” sibling PR alert (same defect as fleet #34)

Posted via lml2468 token. Earlier comment: my R1 APPROVED on this PR did not check the caller repo's GHAS status. yujiawei caught the runtime failure on octo-fleet #34 (#issuecomment-4659753903); the same defect applies here.

This PR's dependency-review.yml is guaranteed-red on every PR because:

  • gh api repos/Mininglamp-OSS/<this-repo> β†’ private: true, security_and_analysis.code_security.status: "disabled"
  • actions/dependency-review-action requires Dependency Graph + GitHub Advanced Security enabled on the caller repo. Without it: ##[error]Dependency review is not supported on this repository.

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):

  • (A) Enable GHAS on this repo. Cost: license fee.
  • (B) Gate the reusable workflow with if: github.event.repository.private == false so private callers no-op. Net +1 line.
  • (C) Drop dependency-review.yml from this caller in a follow-up; keep secret-scan + pr-title-lint. Tradeoff: dependency-review coverage gap on private repos until GHAS lands.

secret-scan.yml and pr-title-lint.yml in this PR are wired correctly. Only dependency-review.yml needs to be resolved before merge β€” once one of A/B/C lands, the PR is good.

CR pending one of A/B/C. Same recommendation on the three sibling PRs.

β€” 齐静ζ˜₯ πŸ“–

@Jerry-Xin Jerry-Xin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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_target branches, unlike the other new PR workflows that target main. If this repository may accept PRs into maintenance branches, consider adding branches: [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_target is used only for PR title metadata/comment automation, with no local checkout or PR code execution in the caller.
  • dependency-review and secret-scan align 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.
@lml2468

lml2468 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed: removed the dependency-review caller. As you flagged, dependency-review-action needs Dependency Graph + GitHub Advanced Security, which this private repo doesn't have β€” it would be a permanent red check. Kept secret-scan + pr-title-lint (both work on private repos); osv-scanner already covers dependency CVEs. Please re-review.

@lml2468

lml2468 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

齐静ζ˜₯ πŸ“– re-review (R2, head 156858cf45f9) β€” verdict: APPROVED

Earlier review: R1 APPROVED on 12c99b7e4fd8, then #issuecomment-4659756702 follow-up CR alert for the dependency-review-on-private-repo failure (one of the 4 PRs in the batch I had to walk back). This R2 verifies the fix.

Diffed 12c99b7e4fd8 β†’ 156858cf45f9. Same fix shape as octo-daemon-cli #33 R2 + octo-fleet #34 R2 β€” author took option C from my sibling alert and yujiawei's CR: drop dependency-review.yml from this private-repo caller, keep secret-scan.yml + pr-title-lint.yml. -14/+0.

🟒 Right fix. octo-version-sync is private: true with security_and_analysis.code_security.status: "disabled"; actions/dependency-review-action would hard-fail every PR. Option C is the same drop the two sibling PRs already landed β€” consistent across the batch.

🟒 secret-scan + pr-title-lint preserved. Byte-identical to siblings, R1 reusable contract verification still holds.

⚠️ Coverage gap acknowledged: no automated dependency-review on private-repo PRs until GHAS lands org-wide or option B (gate the reusable on private == false centrally in Mininglamp-OSS/.github) is implemented. For a version-sync utility repo with a small contributor pool this is reasonable interim state.

Final sibling status (after this lands)

  • claw-channel-octo #23 β€” still has dependency-review.yml, last private-repo PR in the batch needing the same drop
  • octo-daemon-cli #33 β€” already landed option C (head 41bc29a840e5)
  • octo-fleet #34 β€” already landed option C (head 3f6297df7a83)
  • octo-version-sync #25 β€” this PR, landing option C now
  • octo-im #17 (public) β€” unaffected, merged

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 dependency-review.yml knowing it's a no-op without GHAS.

LGTM β€” ship it.

β€” 齐静ζ˜₯ πŸ“–

mochashanyao
mochashanyao previously approved these changes Jun 9, 2026

@mochashanyao mochashanyao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[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-action requires Dependency Graph + GitHub Advanced Security (unavailable on private repos). The existing osv-scanner.yml provides equivalent CVE coverage.
  • Both reusables are well-engineered. The pr-title-lint reusable uses github-script with 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
yujiawei previously approved these changes Jun 9, 2026

@yujiawei yujiawei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 ⚠️ (non-blocking) 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_target in pr-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 an env variable (PR_TITLE) that is never shell- or string-interpolated into the github-script body β€” so there is no script/command-injection path from an attacker-controlled title. It uses actions/github-script SHA-pinned to 3a2844b7… (v9.0.0) and runs with only pull-requests: write. No untrusted-code execution surface.
  • secret-scan.yml β€” SAFE. Uses pull_request (base-context) + push triggers; the reusable checks out with persist-credentials: false, downloads gitleaks as a SHA256-pinned binary (551f6fc8…, v8.30.1, version-mismatch guarded), checkout pinned to de0fac2e… (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-lint does not produce a green check on this PR β€” expected. Because it triggers on pull_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 a pr-title-lint check on this PR is not a problem.
  • Branch-filter consistency (nit). secret-scan.yml filters branches: [main], while pr-title-lint.yml runs on pull_request_target for all base branches. If this repo only takes PRs into main, adding branches: [main] to pr-title-lint would make the two consistent. Purely cosmetic.
  • synchronize in pr-title-lint types (nit). The title cannot change on a synchronize event, so that type is effectively a harmless no-op re-run. Fine to leave as-is for parity with the org template.
  • Mutable @v1 ref. Both callers pin the moving v1 tag (org convention). SHA-pinning would give stronger supply-chain reproducibility, but the .github repo 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 Jerry-Xin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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, and pr-title-lint, completing reusable caller coverage, but the diff only adds .github/workflows/pr-title-lint.yml and .github/workflows/secret-scan.yml. There is no .github/workflows/dependency-review.yml or 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:4 uses pull_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 existing zizmor ignore style used in .github/workflows/labeler.yml.

βœ… Highlights

  • .github/workflows/secret-scan.yml:9-15 keeps permissions minimal and only grants contents: read.
  • .github/workflows/pr-title-lint.yml:7-18 avoids inheriting broad top-level permissions and grants only pull-requests: write to the reusable caller.

@lml2468 lml2468 changed the title ci: complete reusable caller coverage ci: complete reusable caller coverage (private-repo set) Jun 10, 2026
@lml2468

lml2468 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@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.
@lml2468 lml2468 dismissed stale reviews from yujiawei and mochashanyao via e831037 June 10, 2026 11:08
@lml2468

lml2468 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

βœ… Review β€” PR #25 (e831037): CI caller coverage β€” verified, good to merge

Re-reviewed at the latest commit. Two reusable-workflow callers (secret-scan, pr-title-lint), no app code. I verified the security-sensitive bits against the actual reusable definitions rather than the comments. No blocking findings.

The new commit (e831037) β€” pull_request_target safety documentation, and it's accurate

The added comment claims the pull_request_target trigger is safe because the reusable checks out no PR code and reads the title via env. I confirmed all three against Mininglamp-OSS/.github's reusable-pr-title-lint.yml@v1:

  • Title consumed as data, not interpolated β€” the reusable passes PR_TITLE: ${{ inputs.pr_title }} as an env var and reads process.env.PR_TITLE inside github-script; it is not ${{ }}-spliced into the JS body. No script-injection surface. βœ“
  • No code checkout β€” the reusable uses the API only (no actions/checkout), so a malicious fork branch can't execute in the privileged context. βœ“
  • Caller passes the title as a workflow input (data), not into a shell β€” also safe. βœ“
  • Least privilege β€” caller permissions: {} top-level, job scoped to pull-requests: write (needed to post/update the lint comment). βœ“

pull_request_target is the correct trigger here (a plain pull_request from a fork can't comment on the PR), and the usage doesn't introduce the classic PR-target code-exec footgun. The zizmor: ignore[dangerous-triggers] suppression is justified and documented.

secret-scan caller β€” fine

Matches the reusable contract; permissions: {} top-level + contents: read on the job. Triggers on PR + push to main. βœ“

Verified the broader claims

  • Sibling parity is real β€” claw-channel-octo and octo-daemon-cli each carry exactly pr-title-lint.yml + secret-scan.yml and no dependency-review, matching what this PR adds. βœ“
  • Dropping dependency-review is correct β€” dependency-review-action needs the Dependency Graph + GitHub Advanced Security, which private repos lack, so it fails permanently; dependency CVEs are already covered by the repo's existing osv-scanner.yml (confirmed present). This is the false-positive Codex flagged earlier β€” the removal is right. βœ“
  • No duplication β€” neither workflow already exists in this repo's .github/workflows/. βœ“
  • CI: actionlint + workflow-sanity (no tabs) pass; secret-scan itself passes. (check-sprint ❌ is the usual board gate, not code.)

Verdict

Clean, minimal, correctly-scoped CI hardening with sound pull_request_target handling and accurate parity/coverage reasoning. LGTM.

@mochashanyao mochashanyao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[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:
    1. No code checkout: The reusable workflow (reusable-pr-title-lint.yml@v1) uses only actions/github-script (SHA-pinned 3a2844b7). It never calls actions/checkout and never executes PR branch code.
    2. Title via env, not interpolation: pr_title input β†’ env.PR_TITLE β†’ process.env.PR_TITLE in JS. Never shell-interpolated.
    3. Markdown injection mitigated: Title appears in a comment body inside backtick code span, with .replace(//g, "'")` preventing escape.
    4. core.setFailed() logging: Title in log output is informational only, no execution risk.
  • Safety rationale comment (commit 3) documents the pull_request_target decision with zizmor: ignore[dangerous-triggers] β€” matches the pattern in labeler.yml line-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 + push triggers on main only.
  • Delegates entirely to reusable-secret-scan.yml@v1 which: downloads gitleaks with SHA256 verification, uses actions/checkout with persist-credentials: false, scans git history only (no project code execution).
  • permissions: {} at workflow level; job-level contents: read β€” minimal.

3. Suggestions

None blocking. The PR is clean and ready to land.

4. Additional Observations

  • The PR body correctly explains why dependency-review was intentionally excluded (private repos lack GitHub Advanced Security). This is consistent with claw-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 β†’ input pr_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 β†’ input pr_number β†’ reusable input β†’ env.PR_NUMBER β†’ parseInt() β†’ API call issue_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.sha via 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.yml uses pull_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.yml uses pull_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 yujiawei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.yml is byte-identical to the established private-repo pattern. It matches octo-fleet/.github/workflows/secret-scan.yml (a private sibling) line for line: pull_request + push on main, top-level permissions: {}, job-level contents: read, calling reusable-secret-scan.yml@v1.
  • βœ… pr-title-lint.yml is byte-identical to the established pattern. It matches octo-fleet/.github/workflows/pr-title-lint.yml: pull_request_target (types opened/edited/reopened/synchronize), top-level permissions: {}, job-level pull-requests: write, calling reusable-pr-title-lint.yml@v1.
  • βœ… No duplication. Neither secret-scan.yml nor pr-title-lint.yml previously existed in this repo; this genuinely adds new coverage rather than colliding with existing workflows.
  • βœ… pull_request_target is used safely (the main fork-PR risk vector). The reusable reusable-pr-title-lint.yml@v1 checks out NO PR code β€” it reads PR metadata via the API only. The PR title is passed as a workflow_call input, exposed via env: PR_TITLE, and consumed in github-script via process.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-standard zizmor: ignore[dangerous-triggers] annotation correctly document this.
  • βœ… secret-scan callee is supply-chain hardened. reusable-secret-scan.yml@v1 checks out with persist-credentials: false, pins actions/checkout to 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-action requires 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-octo reference 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 @v1 tag pinning. Both callers reference the reusable workflows by the mutable tag @v1 rather than a commit SHA. If @v1 were force-moved in Mininglamp-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 .github org repo's v1 tag, 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-scan workflow's purpose is, in fact, to catch them.

@lml2468

lml2468 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Quick fact-check on the "private repo names" nit

Re 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:

Repo cited Visibility
claw-channel-octo private βœ“
octo-daemon-cli private βœ“
octo-fleet private βœ“
octo-version-sync (this repo) private βœ“

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, cc-channel-octo is the one that's public β€” consistent with it being the public product-line repo β€” which is why its CI set differs.)

So I'd treat that 🟑 as a non-issue; the body doesn't need editing. The substantive verification stands: callers are correct, pull_request_target handling is safe, and dropping dependency-review is right for a private repo (osv-scanner covers dep CVEs). LGTM.

@lml2468

lml2468 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

πŸ“‹ Review-state note β€” one stale CHANGES_REQUESTED on an old SHA

Heads up before merge: the latest review per reviewer is β€”

Reviewer Latest On SHA vs HEAD e831037
Jerry-Xin πŸ”΄ CHANGES_REQUESTED 156858c stale (older commit)
mochashanyao βœ… APPROVED e831037 current
yujiawei βœ… APPROVED e831037 current

Both approvals are on the current HEAD. The one open πŸ”΄ is Jerry-Xin's, still pinned to the older 156858c (the "drop dependency-review" commit), before the later e831037 pull_request_target-safety commit.

That earlier RC was the dependency-review "missing security baseline" finding β€” which is a false positive: dependency-review-action requires the Dependency Graph + GitHub Advanced Security, which private repos don't have, so it fails permanently; dependency CVEs here are already covered by the existing osv-scanner.yml. Removing it is the correct call (and matches the sibling private repos). I verified this in my review above.

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 e831037 and lift the stale RC before this can actually merge. No code change required.

@Jerry-Xin Jerry-Xin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@lml2468 lml2468 merged commit 8d4b1dc into main Jun 10, 2026
15 of 16 checks passed
@lml2468 lml2468 deleted the ci/complete-caller-coverage branch June 10, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants