ci: add CodeQL, Release Please, and dependency age check

[justin-layerv]

Summary

• Add CodeQL workflow (go + actions) — weekly schedule + PR/push triggers
• Add Release Please workflow (go release type)
• Add Go dependency age check (7-day quarantine on newly published modules)

Aligns with NHP repo security baseline.

🤖 Generated with Claude Code

[claude]

Code Review: CI Workflows (CodeQL, Release Please, Dependency Age Check)

Overall this is a well-structured PR that adds meaningful security and release infrastructure. The workflows follow good practices — pinned action SHAs, least-privilege permissions, timeouts, and concurrency controls. Below are specific observations and suggestions.

---

codeql.yml — Looks Good

Strengths:

• Actions pinned to commit SHAs with version comments for auditability
• security-and-quality query suite (broader than the default security-only)
• fail-fast: false ensures one language failure doesn't mask issues in the other
• concurrency group prevents redundant parallel runs on rapid pushes
• Minimal permissions (security-events: write only)

Minor nit:

• The checkout action comment says # v6 while the same SHA in claude-code-review.yml says # v6.0.2. Consider using the same comment style across workflows for consistency.

---

release-please.yml — Looks Good

Clean and minimal. No issues here. The defaults are appropriate for a single-module Go project.

One consideration:

• If the repo ever adds branch protection rules requiring a non-default token for PR creation, you may need to pass a PAT via token:. The default GITHUB_TOKEN works for now but worth keeping in mind.

---

dependency-age-check.yml — Good Concept, A Few Hardening Suggestions

This is the most complex workflow and where I have the most feedback. The 7-day quarantine on new Go modules is a solid supply-chain defense.

Strengths:

• Correctly skips pseudo-versions (v0.0.0-YYYYMMDDHHMMSS-...)
• Proper Go module proxy path encoding (uppercase → ! + lowercase)
• Bypass mechanism via label for emergencies, documented in the error output
• set -euo pipefail for strict error handling
• Minimal contents: read permission

Suggestions:

1. No retry on transient curl failures (medium priority)
   curl -sf -m 10 with no retry means a single transient proxy hiccup or rate-limit response will flag a hard error and block the PR. Consider adding a small retry loop (e.g., 2-3 attempts with a short backoff):

   for attempt in 1 2 3; do
     response=$(curl -sf -m 10 "$info_url" 2>/dev/null || echo "")
     [ -n "$response" ] && break
     sleep 2
   done

   This is especially relevant since the check iterates over potentially many modules serially — if the proxy throttles partway through, it would be frustrating for the PR author.

2. jq dependency is implicit (low priority)
   The script uses jq -r '.Time' but doesn't explicitly ensure jq is available. While ubuntu-latest runners include jq, adding a brief comment or an explicit install step makes the dependency visible and future-proof if GitHub changes the runner image.

3. Consider distinguishing "proxy unreachable" from "module not found" (low priority)
   Currently, an empty curl response triggers the same error whether the proxy is down, the module is private, or the version genuinely doesn't exist. You could check the HTTP status code to differentiate:

   • 404/410 → module/version not found (possibly private — skip or warn differently)
   • 5xx / timeout → transient failure (retry)

   This would reduce noise for repos that mix public and private modules.

4. The age-check-bypass label grants a full bypass (design observation)
   Anyone with label permissions on the repo can skip the entire check. This is probably fine given the intended audience, but if you want an audit trail, consider requiring a PR comment explaining why the bypass was needed (or log the bypass event).

5. Potential edge case with grep '^+[^+]' (very low priority)
   This pattern to extract added lines from the diff works well for go.sum format but would also capture lines that happen to start with + in non-diff contexts (e.g., diff metadata). In practice this is extremely unlikely for go.sum files, but grep -E '^\+[a-z]' would be slightly more precise since all Go module paths start with a lowercase letter.

---

Security Considerations

• Action pinning: All actions are pinned to full commit SHAs — this is the recommended practice and prevents supply-chain attacks via tag manipulation. Good.
• Permissions: All three workflows use appropriately scoped permissions. No over-provisioning detected.
• The dependency age check itself is a supply-chain defense — it provides a quarantine window to catch compromised packages before they're adopted. This complements the existing Dependabot configuration well.

Test Coverage

These are CI workflow files, so traditional unit tests don't apply. However, it would be worth validating the dependency-age-check script on a test PR that adds a known-old dependency and a known-new dependency to confirm both paths (pass and fail) work as expected before merging. The shell script has enough logic that a manual dry run would build confidence.

---

Summary

This is a solid PR. The workflows are well-configured and follow security best practices. The dependency age check is the most novel addition and would benefit from the retry logic and proxy error differentiation mentioned above, but none of the suggestions are blockers. Nice work aligning with the NHP repo security baseline.

[claude]

PR Review: ci: add CodeQL, Release Please, and dependency age check

Overall this is a solid PR that adds three well-structured CI/CD workflows for security scanning, supply chain protection, and automated release management. The code quality is high — nice work. Here's detailed feedback:

---

.github/workflows/codeql.yml — Looks Good

Strengths:

• All actions pinned to commit SHAs with version comments — this is supply chain security best practice and consistent with the existing workflows in this repo.
• Permissions follow least-privilege (actions: read, contents: read, security-events: write).
• Concurrency group with cancel-in-progress: true prevents redundant analysis runs.
• fail-fast: false ensures the Go and Actions analyses are independent.
• security-and-quality query suite is a good choice over the default security-only.
• paths-ignore avoids wasted runs on doc-only changes.

No issues found.

---

.github/workflows/dependency-age-check.yml — Good, with minor suggestions

Strengths:

• Minimal permissions (contents: read).
• age-check-bypass label provides a clear escape hatch for emergencies.
• Correctly skips pseudo-versions (v0.0.0-{timestamp}-{hash}).
• Proper Go module proxy encoding for uppercase letters (line 63).
• set -euo pipefail for strict error handling.
• fetch-depth: 0 correctly set for the three-dot diff.

Suggestions:

1. Go proxy flakiness could cause spurious failures (minor): curl -sf -m 10 with a fallback to empty string means a transient proxy.golang.org outage will fail the check. Consider adding a single retry:

   response=$(curl -sf -m 10 "$info_url" 2>/dev/null || sleep 2 && curl -sf -m 10 "$info_url" 2>/dev/null || echo "")

   Or use --retry 2 --retry-delay 2 (curl's built-in retry). This is non-blocking — the current behavior (fail-closed) is the safer default for a security check, but it could create friction if the proxy has a blip.

2. jq dependency is implicit (nit): The script relies on jq being available on the runner. It is pre-installed on ubuntu-latest, but worth a brief comment for clarity — or add a verification step. If this ever runs on a self-hosted runner, it would silently fail.

3. The go.sum path glob on the trigger only matches **/go.sum — I see the repo also has docker/web-app/go.mod but no go.sum there. If one appears later, the workflow will pick it up automatically thanks to the **/go.sum glob and the git diff detection. Good forward-compatible design.

4. Minor edge case with while read loop (very minor): If a go.sum diff line contains a module path with spaces (essentially impossible for Go modules, but worth noting), the IFS=' ' split would misbehave. Not a real concern for Go, just a defensive coding note.

---

.github/workflows/release-please.yml — Good, one consideration

Strengths:

• Pinned to v4.4.0 SHA.
• Correct permissions (contents: write, pull-requests: write).
• release-type: go is correct for this project.
• changelog-path: CHANGELOG.md makes the output location explicit.

Consideration:

1. Token permissions with branch protection: The workflow uses the implicit GITHUB_TOKEN. If the main branch has branch protection rules requiring status checks or specific approvals, the default token may not be able to merge the Release Please PR or create the release. If that becomes an issue, you'd need a GitHub App token or PAT. Fine as-is if branch protection is permissive for the bot.

2. Future-proofing: For a single-module Go repo, the inline config is clean. If the project grows (e.g., multi-module), you'd want to migrate to .release-please-manifest.json + release-please-config.json. No action needed now.

---

Security Assessment

• Action pinning: All three workflows pin actions to full commit SHAs. This is the gold standard for preventing supply chain attacks via tag mutation.
• Permissions: All workflows follow least-privilege. No excessive write permissions.
• Dependency age check: The 7-day quarantine is a meaningful defense against dependency confusion and typosquatting attacks where malicious packages are published and consumed quickly before detection. The age-check-bypass label is appropriately named and documented in the error output.
• No secret exposure risks: No secrets are passed to shell scripts or logged.

Test Coverage

These are CI workflow files, so traditional unit tests don't apply. To validate:

• CodeQL: Will self-test on the next push to main or PR targeting main.
• Dependency age check: Could be tested by opening a PR that bumps a dependency. The workflow_dispatch trigger also allows manual testing.
• Release Please: Will activate on the next push to main with conventional commits.

---

Summary

Workflow	Verdict
codeql.yml	Approve as-is
dependency-age-check.yml	Approve — optional curl retry improvement
release-please.yml	Approve as-is

This PR aligns well with security baselines. The workflows are well-structured, secure, and production-ready. Ship it.

[claude]

Code Review: CI Workflow Additions

Good PR — well-structured workflows with solid security defaults. The SHA-pinned actions, least-privilege permissions, and job timeouts across all three files are exactly right. Here's detailed feedback:

---

.github/workflows/codeql.yml — Looks Good

Strengths:

• SHA-pinned actions with version comments for auditability
• security-and-quality query suite is the right choice over the default security-only
• paths-ignore avoids wasting compute on doc-only changes
• fail-fast: false ensures one language failure doesn't mask the other
• Concurrency group with cancel-in-progress prevents redundant runs

Nit (non-blocking):

• The schedule trigger ignores paths-ignore filters (GitHub always runs scheduled workflows on the default branch). This is fine — weekly full scans are desirable — but worth a comment so future readers don't think paths-ignore applies to cron runs too.

---

.github/workflows/dependency-age-check.yml — Solid, a Few Items to Consider

Strengths:

• The 7-day quarantine is a good supply chain defense
• Pseudo-version skip (v0.0.0-<timestamp>-) is correct — these are commit-pinned and don't go through the normal publish flow
• The Go module proxy URL encoding (! + lowercase for uppercase chars) is properly handled
• age-check-bypass label escape hatch with clear messaging is well thought out
• workflow_dispatch for manual testing is a nice touch
• Fail-closed on curl/parse errors (treats unverifiable deps as failures) is the right security posture

Suggestions:

1. workflow_dispatch without go.sum context (medium): When triggered manually via workflow_dispatch, github.event.pull_request.base.ref will be empty and the fallback 'main' kicks in. But git diff "origin/main...HEAD" on the default branch will show no diff. Consider adding a note or input parameter for the manual trigger so operators know to run it on a branch, not on main.

2. Sequential curl calls could be slow (low): Each dependency is checked one-at-a-time with a 10s timeout. A PR adding 20+ new transitive deps could take minutes. Not a blocker — the 10-minute job timeout is a safety net — but worth noting for future optimization (e.g., xargs -P for parallel requests).

3. sort -u deduplication subtlety (low): The sort -u after sed 's|/go.mod$||' deduplicates lines like module v1.2.3 (from the hash line and the go.mod line). This is correct behavior — just noting it works as intended.

---

.github/workflows/release-please.yml — Works, One Consideration

Strengths:

• Minimal and clean
• SHA-pinned with version comment
• Correct permissions for creating release PRs and GitHub releases

Consideration:

1. No token override (medium): The action defaults to GITHUB_TOKEN. Releases and PRs created with GITHUB_TOKEN won't trigger other workflows (e.g., the CodeQL workflow won't run on the release merge commit, though the weekly schedule covers that). If you want release merges to trigger CI, you'd need a PAT or GitHub App token. This may be intentional — just flagging it.

2. Missing config files (low): Release Please v4 can auto-bootstrap without release-please-config.json / .release-please-manifest.json, but adding them gives you explicit control over versioning, changelog sections, and component names. Not required for a single-component Go repo, but recommended as the project grows.

---

Overall Security Assessment

• All actions SHA-pinned — good
• Permissions follow least-privilege — good
• No secrets exposed in logs — good
• The dependency age check adds meaningful supply chain protection
• Fail-closed error handling in the age check is the right default

Test Coverage

These are CI-only changes (no application code), so unit tests don't apply. The workflows will be exercised by this PR itself (CodeQL and dependency-age-check should run). Consider verifying that both triggered successfully before merging.

Verdict: Approve with minor suggestions. The workflows are well-crafted and follow CI/CD best practices. The items above are improvements, not blockers.