ci: add OSV-Scanner vulnerability scanning

[lml2468]

What

Add OSV-Scanner dependency vulnerability scanning.

How

Uses Google's official reusable workflows (v2.3.8, SHA-pinned to 9a498708...):

• PR scan (osv-scanner-reusable-pr.yml): incremental — only reports new vulns introduced by PR
• Scheduled/push scan (osv-scanner-reusable.yml): full scan on push to main, weekly Monday 04:30 UTC, and manual dispatch

Results uploaded as SARIF to GitHub Security > Code Scanning tab.

Permissions

Minimal: actions: read, contents: read, security-events: write.

Ref: Workflow optimization task T10

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[yujiawei]

Code Review — PR #71 (octo-admin)

Summary

This PR adds a single GitHub Actions workflow (.github/workflows/osv-scanner.yml, 34 lines) that wires up Google's OSV-Scanner via the official reusable workflows. The implementation follows CI security best practices and I found no blocking issues. Verdict: APPROVED.

Verification

Item	Result	Evidence
Action SHA pinned, not floating tag	✅	Both jobs pin @9a498708959aeaef5ef730655706c5a1df1edbc2; the # v2.3.8 comment is accurate — refs/tags/v2.3.8 dereferences to exactly that commit.
Reusable workflow paths exist at pinned SHA	✅	osv-scanner-reusable.yml and osv-scanner-reusable-pr.yml both present in google/osv-scanner-action at the pinned commit.
Permissions are minimal & correct	✅	actions: read, contents: read, security-events: write — matches exactly what the upstream reusable workflows declare (the security-events: write is required for the SARIF upload to Code Scanning).
Event routing has no overlap/gaps	✅	scan-scheduled gated on push || schedule || workflow_dispatch; scan-pr gated on pull_request || merge_group. No event triggers both jobs.
Uses pull_request (not pull_request_target)	✅	Avoids the classic untrusted-code-with-write-token footgun. The upstream full-scan workflow also sets persist-credentials: false on checkout.
Cron schedule	✅	30 4 * * 1 = Monday 04:30 UTC, matches the "weekly Monday 04:30 UTC / 12:30 CST" comment.

Findings

No P0 or P1 issues. The items below are non-blocking suggestions (P2 / nits) — none of them need to be addressed before merge.

P2 — merge_group routing relies on GITHUB_BASE_REF, which is empty in merge-queue events

scan-pr runs the incremental reusable workflow (osv-scanner-reusable-pr.yml) for merge_group events. That reusable workflow does git checkout $GITHUB_BASE_REF to establish the "before" baseline, but GITHUB_BASE_REF is only populated for pull_request events — it is empty in merge_group runs. The practical effect is that the before/after diff degenerates (both scans see the same tree), so the merge-queue scan becomes a no-op rather than a meaningful incremental check. It won't produce false positives or hard-fail the queue, so this is informational. If you want a real scan in the merge queue, route merge_group to the full scan-scheduled job instead. If the repo doesn't use GitHub merge queues, the merge_group trigger simply never fires and this is moot.

P2 — fail-on-vuln defaults to true (intended, but worth a heads-up for maintainers)

Both reusable workflows default fail-on-vuln: true. Consequences worth being aware of:

• The PR job will block a PR that introduces a new vulnerable dependency (this is the desired gate).
• The push/scheduled job will fail on push to main if any known vuln exists in the dependency tree — including a newly-published advisory against an already-merged dependency. That can turn main red without a code change. This is standard OSV-Scanner behavior and a reasonable default, but flagging it so the team isn't surprised. Set fail-on-vuln: false on scan-scheduled if you'd prefer scheduled runs to report-only.

Nit — consider concurrency to cancel superseded runs

Adding a concurrency block (e.g. group per ref, cancel-in-progress: true) would avoid redundant scans on rapid pushes. Pure optimization, optional.

Notes for the human reviewer (security-sensitive)

Nothing here needs manual security sign-off beyond confirming policy intent:

• Supply-chain hygiene is correct: third-party action is SHA-pinned and the SHA was verified to equal the v2.3.8 tag.
• The only elevated permission is security-events: write, which is the minimum required to upload SARIF results; no secrets, tokens, or write access to repo contents are granted.
• Confirm the team is comfortable with fail-on-vuln: true making this a blocking merge gate (see P2 above).

[Jerry-Xin]

The PR is relevant to octo-admin, but the merge queue path can silently validate the wrong diff.

🔴 Blocking

🔴 Critical — .github/workflows/osv-scanner.yml:29: scan-pr runs for merge_group, but the called OSV PR reusable workflow checks out $GITHUB_BASE_REF to scan the target branch. GitHub only sets GITHUB_BASE_REF for pull_request and pull_request_target, not merge_group; on merge_group, git checkout $GITHUB_BASE_REF becomes git checkout, which exits successfully without switching branches. That makes the “old” and “new” scans run against the same merge-group commit, so new vulnerabilities can be missed while the required merge-queue check passes. Remove merge_group from this job, use a full scan for merge_group, or wrap OSV with a workflow that resolves the merge-group base ref explicitly. (docs.github.com)

💬 Non-blocking

🟡 Warning — .github/workflows/osv-scanner.yml:3: Consider matching the repository’s existing PR workflow conventions by skipping draft PRs or handling ready_for_review, as .github/workflows/ci.yml already does. This is not required for correctness, but it avoids extra security scans on drafts and keeps CI behavior consistent.

✅ Highlights

🔵 Suggestion — .github/workflows/osv-scanner.yml:14: The permissions are appropriately minimal for SARIF upload.

🔵 Suggestion — .github/workflows/osv-scanner.yml:22 and .github/workflows/osv-scanner.yml:30: Pinning the reusable workflow to the exact v2.3.8 commit is a good supply-chain control.

[yujiawei]

Code Review — PR #71 (octo-admin)

Summary

This PR adds a single GitHub Actions workflow (.github/workflows/osv-scanner.yml, 32 lines) that wires up Google's OSV-Scanner via the official SHA-pinned reusable workflows. The previously-raised blocking issue (the merge_group trigger causing a silent no-op scan) has been fixed in the latest commit. I found no remaining blocking issues. Verdict: APPROVED.

Verification

Item	Result	Evidence
Prior merge_group blocker resolved	✅	Commit 114e118a ("fix: remove merge_group trigger") removed merge_group from both on: and the scan-pr condition. The upstream osv-scanner-reusable-pr.yml does git checkout $GITHUB_BASE_REF (line 76), which is empty on merge_group events — so removing it was the correct fix. The current diff contains no merge_group.
Action SHA pinned, not floating tag	✅	Both jobs pin @9a498708959aeaef5ef730655706c5a1df1edbc2; verified refs/tags/v2.3.8 resolves to exactly that commit, so the # v2.3.8 comment is accurate.
Reusable workflow paths exist at pinned SHA	✅	osv-scanner-reusable.yml and osv-scanner-reusable-pr.yml both confirmed present in google/osv-scanner-action at the pinned commit.
Permissions are minimal & correct	✅	actions: read, contents: read, security-events: write — exactly matches what the upstream reusable workflows declare. security-events: write is the minimum required for the SARIF upload to Code Scanning.
Event routing has no overlap/gaps	✅	scan-scheduled gated on push || schedule || workflow_dispatch; scan-pr gated on pull_request. The on: triggers (pull_request/push on main, schedule, workflow_dispatch) map 1:1 — no event fires both jobs, none is unhandled.
Uses pull_request (not pull_request_target)	✅	Avoids the classic untrusted-code-with-write-token footgun.
Cron schedule	✅	30 4 * * 1 = Monday 04:30 UTC, matches the "weekly Monday 04:30 UTC / 12:30 CST" comment.

Findings

No P0 or P1 issues. The items below are non-blocking suggestions — none need to be addressed before merge.

P2 — scan-pr uses bare pull_request; consider matching ci.yml event-type conventions

The repo's ci.yml intentionally declares types: [opened, reopened, synchronize, ready_for_review, converted_to_draft], with an in-file note that converted_to_draft is kept so required-status checks don't deadlock when a PR is demoted to draft. This OSV workflow uses bare pull_request (default types: opened, synchronize, reopened). Two practical consequences worth a heads-up:

• It will run on draft PRs (extra scans on drafts).
• If OSV-Scanner / scan-pr is ever made a required status check, toggling a PR's draft state could reproduce the exact "stale pending" deadlock that ci.yml's comment was written to avoid.

If you don't intend to make this a required check and don't mind scanning drafts, this is moot. Otherwise, consider aligning the types: list with ci.yml.

P2 — fail-on-vuln defaults to true (intended, but worth confirming)

Both reusable workflows default fail-on-vuln: true:

• The PR job will block a PR that introduces a new vulnerable dependency (the desired gate).
• The push/scheduled job will fail on push to main (and on the weekly cron) if any known vuln exists in the dependency tree — including a newly-disclosed advisory against an already-merged dependency. That can turn main red without a code change. This is standard OSV-Scanner behavior and a reasonable default; flagging it so the team isn't surprised. Set fail-on-vuln: false on scan-scheduled if you'd prefer scheduled/push runs to be report-only.

Nit — consider a concurrency block

ci.yml already uses concurrency with cancel-in-progress: true. Adding the same here would cancel superseded scans on rapid pushes. Pure optimization, optional.

Notes for the human reviewer (security-sensitive)

Nothing here needs manual security sign-off beyond confirming policy intent:

• Supply-chain hygiene is correct: the third-party reusable workflow is SHA-pinned and the SHA was verified to equal the v2.3.8 tag.
• The only elevated permission is security-events: write, the minimum required to upload SARIF; no secrets, tokens, or write access to repo contents are granted.
• Confirm the team is comfortable with fail-on-vuln: true making the PR scan a blocking merge gate, and decide whether scheduled/push failures-on-main are acceptable (see P2 above).

[Jerry-Xin]

Adds relevant dependency vulnerability scanning for this repository, but the PR workflow has a forked-PR failure mode that should be fixed before merge.

🔴 Blocking

• 🔴 Critical: osv-scanner.yml calls the OSV PR reusable workflow with its default SARIF upload enabled. Forked pull_request runs receive a read-only GITHUB_TOKEN regardless of requested permissions, so security-events: write is unavailable and the upload step can fail for external contributors. This would make forked PRs fail because of Code Scanning upload permissions rather than actual vulnerabilities. GitHub documents fork PR tokens as read-only, and OSV’s workflow requires security-events: write for SARIF upload. (github.blog)

  Suggested fix: disable SARIF upload for forked PRs, for example by passing upload-sarif: ${{ github.event.pull_request.head.repo.full_name == github.repository }} to scan-pr, or split same-repo PR SARIF upload from fork-safe PR annotation/scanning behavior.

💬 Non-blocking

• 🔵 Suggestion: osv-scanner.yml sets workflow-level security-events: write. Consider moving permissions to each job, matching the existing codeql.yml style, so the write scope is granted only where it is usable.

✅ Highlights

• ✅ The PR passes the project relevance gate: dependency vulnerability scanning is directly relevant to this Node/Vite application with package-lock.json.
• ✅ The external reusable workflow is SHA-pinned, and the SHA matches v2.3.8.
• ✅ The full scan coverage on push, weekly schedule, and manual dispatch is appropriate for dependency vulnerability monitoring.

[Jerry-Xin]

Relevance gate: passed. This PR adds dependency vulnerability scanning for the octo-admin Node project and is in scope for the repository.

🔴 Blocking: None.

💬 Non-blocking:

• 🔵 Suggestion — .github/workflows/osv-scanner.yml:3-10: Consider adding concurrency similar to existing CI/CodeQL workflows so repeated pushes to the same PR cancel older OSV runs and avoid stale scan noise.
• 🔵 Suggestion — .github/workflows/osv-scanner.yml:4-5: If draft PR cost/noise matters, consider matching the existing CI pattern and skipping scans for draft PRs.

✅ Highlights:

• .github/workflows/osv-scanner.yml:12-15: Permissions are appropriately narrow for SARIF upload.
• .github/workflows/osv-scanner.yml:20,28: The reusable OSV workflows are SHA-pinned, and the SHA matches v2.3.8.
• .github/workflows/osv-scanner.yml:36: Fork PR SARIF upload is disabled, which avoids the common read-only token failure while preserving workflow output.

[yujiawei]

Code Review — PR #71 (octo-admin)

Summary

This PR adds a single GitHub Actions workflow (.github/workflows/osv-scanner.yml, 36 lines) wiring up Google's OSV-Scanner via the official SHA-pinned reusable workflows. The two blocking issues raised on earlier commits — the merge_group silent no-op scan, and the fork-PR SARIF-upload permission failure — have both been fixed in the current head. I verified each fix against the upstream pinned workflow and found no remaining blocking issues. Verdict: APPROVED.

Verification

Item	Result	Evidence
Prior merge_group blocker resolved	✅	Commit 114e118a removed merge_group from both on: and the scan-pr if:. The upstream osv-scanner-reusable-pr.yml does git checkout $GITHUB_BASE_REF, which is empty on merge_group events; removing the trigger was the correct fix. Current diff contains no merge_group.
Fork-PR SARIF blocker resolved	✅	Commit 9ae4186 passes upload-sarif: ${{ github.event.pull_request.head.repo.full_name == github.repository }} to scan-pr (osv-scanner.yml:35). Upstream gates the upload step on inputs.upload-sarif == true, so fork PRs (read-only GITHUB_TOKEN) now skip the upload and won't fail on the missing security-events: write. Same-repo PRs still upload. The scan itself still runs and annotates.
Action SHA pinned, not floating tag	✅	Both jobs pin @9a498708959aeaef5ef730655706c5a1df1edbc2; refs/tags/v2.3.8 dereferences to exactly that commit, so the # v2.3.8 comment is accurate.
Reusable workflow paths exist at pinned SHA	✅	osv-scanner-reusable.yml and osv-scanner-reusable-pr.yml both confirmed present in google/osv-scanner-action at the pinned commit.
Permissions are minimal & correct	✅	actions: read, contents: read, security-events: write — matches exactly what both upstream reusable workflows declare. security-events: write is the minimum required for SARIF upload to Code Scanning.
Event routing has no overlap/gaps	✅	scan-scheduled gated on push || schedule || workflow_dispatch; scan-pr gated on pull_request. The on: triggers map 1:1 — no event fires both jobs, none is unhandled.
Uses pull_request (not pull_request_target)	✅	Avoids the classic untrusted-code-with-write-token footgun.
Cron schedule	✅	30 4 * * 1 = Monday 04:30 UTC, matches the "weekly Monday 04:30 UTC / 12:30 CST" comment.

Findings

No P0 or P1 issues. The items below are non-blocking suggestions — none need to be addressed before merge.

P2 — workflow-level security-events: write vs. per-job scoping

Permissions are declared at the workflow level (osv-scanner.yml:11-14). The repo's codeql.yml instead sets permissions: {} at the top and grants security-events: write only on the job that needs it. Since the elevated scope is only consumed inside the reusable workflows (which redeclare their own permissions anyway), moving the grant to each uses: job here would tighten least-privilege and match the existing codeql.yml convention. Functionally equivalent today; purely a hardening/consistency nit.

P2 — scan-pr uses bare pull_request; consider matching ci.yml event-type conventions

ci.yml declares types: [opened, reopened, synchronize, ready_for_review, converted_to_draft], with an in-file note that converted_to_draft is kept so required-status checks don't deadlock when a PR is demoted to draft. This workflow uses bare pull_request (default types: opened, synchronize, reopened). Consequences: it runs on draft PRs, and if scan-pr is ever made a required check, toggling draft state could reproduce the "stale pending" deadlock that ci.yml's comment was written to avoid. Moot if you don't intend this to be a required check and don't mind scanning drafts; otherwise align the types: list.

P2 — fail-on-vuln defaults to true (intended, but worth confirming)

Both reusable workflows default fail-on-vuln: true:

• The PR job will block a PR that introduces a new vulnerable dependency (the desired gate).
• The push/scheduled job will fail on push to main and on the weekly cron if any known vuln exists in the dependency tree — including a newly-disclosed advisory against an already-merged dependency. That can turn main red without a code change. This is standard OSV-Scanner behavior and a reasonable default; flagging it so the team isn't surprised. Set fail-on-vuln: false on scan-scheduled if you'd prefer scheduled/push runs to be report-only.

Nit — consider a concurrency block

ci.yml and codeql.yml both use concurrency with cancel-in-progress: true. Adding the same here would cancel superseded scans on rapid pushes. Pure optimization, optional.

Notes for the human reviewer (security-sensitive)

Nothing here needs manual security sign-off beyond confirming policy intent:

• Supply-chain hygiene is correct: the third-party reusable workflow is SHA-pinned and the SHA was verified to equal the v2.3.8 tag.
• The only elevated permission is security-events: write, the minimum required to upload SARIF; no secrets, tokens, or write access to repo contents are granted. Uses pull_request, not pull_request_target, so untrusted PR code never runs with a write token.
• The fork-PR fix correctly degrades to scan-only (no upload) for external contributors, so the merge gate won't produce permission-based false failures.
• Confirm the team is comfortable with fail-on-vuln: true making the PR scan a blocking merge gate, and decide whether scheduled/push failures on main are acceptable (see P2 above).

[Jerry-Xin]

The PR is relevant to Mininglamp-OSS/octo-admin: adding dependency vulnerability scanning is repository maintenance/security functionality and fits the project scope.

🔴 Blocking

None.

💬 Non-blocking

🟡 Warning: .github/workflows/osv-scanner.yml:4 The PR scan runs for every pull request to main, including documentation-only changes. This is safe, but it may add unnecessary CI load. Consider adding paths / paths-ignore similar to the existing CodeQL workflow if CI cost or noise becomes an issue.

🔵 Suggestion: .github/workflows/osv-scanner.yml:18 Consider adding concurrency for PR scans so superseded runs are canceled when a PR is updated. This would align with the existing CodeQL workflow behavior and reduce queue time.

✅ Highlights

🔵 The reusable workflow references are SHA-pinned while documenting the intended version, which is good supply-chain hygiene.

🔵 The fork/Dependabot SARIF guard at .github/workflows/osv-scanner.yml:37 correctly avoids upload attempts when the token is expected to be read-only.

🔵 Permissions are limited to the capabilities needed for checkout, action metadata access, and Code Scanning upload.

[yujiawei]

Code Review — PR #71 (octo-admin)

Verdict: APPROVED. This adds a single new GitHub Actions workflow (.github/workflows/osv-scanner.yml, +37) that wires OSV-Scanner into PR, push, weekly schedule, and manual triggers. The workflow is correct, follows supply-chain and least-privilege best practices, and has no blocking defects. The notes below are advisory (rollout decisions and hardening nits), not merge blockers.

Verification results

Check	Result	Evidence
SHA pin authenticity	✅	Tag v2.3.8 resolves to commit 9a498708959aeaef5ef730655706c5a1df1edbc2 (GitHub git-refs API + release page). The pin is byte-for-byte correct and is a full 40-char SHA, not a mutable tag.
Reusable workflow paths exist	✅	Both osv-scanner-reusable.yml and osv-scanner-reusable-pr.yml are present under .github/workflows at v2.3.8.
Inputs valid & type-matched	✅	scan-args (string) and upload-sarif (boolean) both exist on the called workflows with matching types. No required input was omitted (every upstream input has a default). No secrets: block is declared upstream, so omitting secrets: inherit is correct.
Permissions least-privilege	✅	actions: read, contents: read, security-events: write is exactly the set needed for SARIF upload to Code Scanning. Permissions are declared at the caller top level (correct — a uses: reusable workflow inherits caller permissions and can only downscope).
Trigger gating	✅	scan-pr (pull_request) and scan-scheduled (push/schedule/workflow_dispatch) are mutually exclusive and exhaustive across all four triggers — no event runs both jobs, none runs zero.
Fork / untrusted-code safety	✅	Uses pull_request (not pull_request_target), so fork PRs run with a read-only GITHUB_TOKEN and no secret access. scan-scheduled (with its write grant) is never reachable from fork-controlled code.
upload-sarif guard	✅	head.repo.full_name == github.repository && user.login != 'dependabot[bot]' correctly disables SARIF upload for fork PRs (read-only token) and Dependabot PRs. The expression only ever evaluates inside the PR-gated job, so there is no null-pull_request edge case. The dependabot[bot] login string is correct.
Cron correctness	✅	30 4 * * 1 is a valid 5-field expression: 04:30 UTC Monday.
YAML correctness	✅	Consistent indentation, well-formed `

Advisory notes (non-blocking)

1. Day-one gating risk if scan-pr is made a required check.
-r ./ recursively scans the entire pre-existing dependency tree, and the PR reusable workflow fails the job when vulnerabilities are found (fail-on-vuln defaults to true upstream). If you promote this check to required in branch protection immediately, the first PR afterward can be blocked by pre-existing vulnerabilities unrelated to its diff. Recommendation: land non-required, observe a cycle or two to establish a clean baseline (or add an osv-scanner.toml ignore list), then promote to required.

2. Required-check name is not what this file shows.
Because the jobs call reusable workflows via uses:, the status check that appears in the PR UI is the inner job name (e.g. scan-pr / osv-scan), not scan-pr. Whoever configures branch protection must pin that inner name; it can change if the reusable workflow is updated upstream.

3. No concurrency: control (P2).
Rapid pushes to an open PR will queue redundant scans rather than canceling superseded runs. Consider a concurrency group with cancel-in-progress: true, scoped so it does not cancel scheduled/push-to-main runs.

4. No job-level timeout — and none is possible here.
timeout-minutes cannot be set on a job that uses uses:, so a hung scan can run to the default ceiling. This is an accepted GitHub limitation, not a fixable defect; a one-line comment in the file would save a future maintainer from trying.

5. Nits.

• The # ... (12:30 CST) comment is ambiguous — CST reads as US Central (UTC−6) to many. The math is right for China (UTC+8); spelling it 12:30 China time (UTC+8) avoids a future "fix" of a non-bug.
• -r ./ from repo root will also scan any vendored/fixture lockfiles. If the first scan surfaces noise from deps you don't ship, scope scan-args to specific paths or add an ignore file.

Human verification suggestions (security-sensitive)

• Confirm the intended branch-protection rollout (note 1/2) before flipping scan-pr to required.
• Confirm the pinned v2.3.8 is the intended version of google/osv-scanner-action (it is the current release line; pin is authentic).

Nice work — the fork/dependabot guard and the full-SHA pin are exactly the details that are easy to get wrong here, and both are correct.

[Jerry-Xin]

The PR is relevant to this repository and adds a scoped CI security workflow for the existing npm dependency surface.

🔴 Blocking: None.

💬 Non-blocking:

• 🔵 Suggestion — .github/workflows/osv-scanner.yml:4: Consider aligning PR trigger behavior with existing CI/CodeQL workflows by explicitly listing PR activity types such as opened, reopened, synchronize, and ready_for_review. This is mainly useful if OSV-Scanner becomes a required check.
• 🔵 Suggestion — .github/workflows/osv-scanner.yml:17: Consider adding concurrency for PR scans so repeated pushes cancel stale OSV runs, matching the repo’s existing CI pattern and reducing redundant workflow load.

✅ Highlights:

• Uses SHA-pinned reusable workflows.
• Permissions are appropriately narrow for checkout and SARIF upload.
• Fork and Dependabot PR SARIF upload handling avoids the common security-events: write permission failure path.
• Full scans on main are non-blocking while PR scans remain blocking for newly introduced vulnerabilities.

[yujiawei]

Code Review — PR #71 (octo-admin)

Summary

Adds a single GitHub Actions workflow (.github/workflows/osv-scanner.yml, +39 lines) that wires up OSV-Scanner dependency vulnerability scanning via Google's official reusable workflows. The change is well-scoped, correctly pinned, and follows upstream guidance closely.

Verdict: APPROVED — no blocking issues found.

Verification

I verified the following against the upstream action at the exact pinned commit:

• ✅ SHA pin is accurate. google/osv-scanner-action@9a498708959aeaef5ef730655706c5a1df1edbc2 resolves exactly to tag v2.3.8. The commit exists upstream. Pinning to a full commit SHA (rather than a floating tag) is the correct supply-chain hardening practice for third-party actions.
• ✅ All inputs are valid at the pinned SHA. scan-args and fail-on-vuln exist in osv-scanner-reusable.yml; scan-args and upload-sarif exist in osv-scanner-reusable-pr.yml. No typos or non-existent inputs.
• ✅ Permissions are minimal and correct (actions: read, contents: read, security-events: write) and match exactly what the reusable workflows declare they need for SARIF upload to the Code Scanning tab.
• ✅ Fork-PR safety is handled correctly. upload-sarif on the PR job is gated to head.repo.full_name == github.repository && user.login != 'dependabot[bot]'. This prevents SARIF-upload failures on fork PRs (where security-events: write is not granted to the fork token) and on dependabot PRs, which is the recommended pattern.
• ✅ Job dispatch is correct. The scan-scheduled/scan-pr split via if: github.event_name == ... cleanly routes pull_request → incremental PR scan and push/schedule/workflow_dispatch → full scan. No event will trigger both or neither.
• ✅ fail-on-vuln: false on the full scan is a deliberate, well-documented choice (inline comment explains pre-existing vulns are tracked via the Security tab, while new vulns are still blocked at PR time since scan-pr inherits the fail-on-vuln: true default). This is sound — it avoids permanently red-ing main on legacy debt while still gating new debt.
• ✅ Repo is public, so Code Scanning / SARIF upload is available without GHAS, and codeql.yml already exists in the repo so Code Scanning is active.
• ✅ Cron 30 4 * * 1 = Monday 04:30 UTC; the inline comment (12:30 CST) is accurate (UTC+8).

Security note (security-sensitive classification)

Nothing here introduces credential exposure or privilege escalation. The workflow grants no write access beyond security-events: write, the third-party action is SHA-pinned, and SARIF upload is correctly suppressed for untrusted fork contexts. The PR scan runs on pull_request (not pull_request_target), so untrusted PR code never executes with elevated token scope. No manual security follow-up required.

Non-blocking suggestions (nits)

These are optional and do not block merge:

• nit (cost/cleanliness): Consider adding a concurrency group (e.g. group: osv-scanner-${{ github.ref }}, cancel-in-progress: true) so superseded PR pushes cancel in-flight scans. Minor CI-minutes saving.
• nit (observability): The pinning comment # v2.3.8 is helpful; consider also documenting in the repo (or relying on Dependabot's github-actions ecosystem) how this SHA gets bumped over time, so the pin doesn't silently go stale.

Neither affects correctness or security.