ci: security scanning suite and governance (consolidates #305-310)#1314
Conversation
|
The suite is thoughtfully structured, but I would not merge it in the current branch state. First blocker: the branch is stale. The direct two-dot diff includes already-merged template/Cookbook/email/skills/vault/markdown files. Please rebase onto latest Second thing to coordinate: #1316 adds Also worth validating after rebase: run |
Pins actions/github-script@v7 to its exact commit SHA as required by the workflow-security gate in odysseus-dev#1314 (zizmor --offline --min-severity=low). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pins actions/github-script@v7 to its exact commit SHA as required by the workflow-security gate in odysseus-dev#1314 (zizmor --offline --min-severity=low). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pins actions/github-script@v7 to its exact commit SHA as required by the workflow-security gate in odysseus-dev#1314 (zizmor --offline --min-severity=low). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the careful read. Lint results against current actionlint is clean across all five workflows (exit 0). hadolint against the existing On the rebase: the branch is purely additive — eight new files, no path overlap with On #1316: small correction — it's already SHA-pinned ( Sequencing: #1314 has no hard dependency on #1015 (different files, no conflict), so it can merge in either order. It composes with #1015 logically. Your call on order — just let me know and I'll rebase to match. |
|
Closing — unsolicited CI/workflow additions (security scanning suites, dev tooling installs, build workflows, packaging targets) need an issue-first discussion. Project CI direction is intentional and not crowdsourced. Per CONTRIBUTING.md, open an issue describing the gap before opening a tooling PR. |
|
Reopening — the previous close was an agent batch acting on a new "issue-first for CI/tooling" rule, applied retroactively to PRs already in active maintainer review. You'd been thoughtfully responding to my rebase request and the conversation was clearly in progress. Picking up from your June 2 reply with the actionlint/hadolint results — I'll re-read and respond. Sorry for the noise. |
|
Thanks for pushing this and for the careful follow-up. Decision from me: approved in direction. Use this security gate scope as the baseline: secret scan, workflow security, dependency review / pip-audit, container scan, and CodeQL with the blocking/advisory split documented here. Please rebase on the current target branch when ready so the final diff is only the intended governance/security files, then we can use this as the security baseline for branch protection. I also agree that security-sensitive reports should go through private disclosure first rather than public issues/discussions until triaged. |
0b76862 to
62a6cce
Compare
b5ec21b to
6e33a91
Compare
Pin actions to commit SHAs, set persist-credentials: false on every checkout, and scope token permissions to the jobs that use them. Suppress the two findings that are safe by design: the description bot's pull_request_target trigger (no fork code runs) and an intentional word-split in the docker manifest step. Clears actionlint and zizmor against dev so the blocking gate from odysseus-dev#1314 can pass once both land.
Pin actions to commit SHAs, set persist-credentials: false on every checkout, and scope token permissions to the jobs that use them. Suppress the two findings that are safe by design: the description bot's pull_request_target trigger (no fork code runs) and an intentional word-split in the docker manifest step. Clears actionlint and zizmor against dev so the blocking gate from #1314 can pass once both land.
Consolidates the security CI work into one reviewable change. Adds, as
separate workflow files under .github/workflows/:
- secret-scan.yml gitleaks (pinned + checksum-verified), full history
- workflow-security.yml actionlint + zizmor, audits the workflows themselves
- dependency-review.yml PR dependency gate + advisory pip-audit
- container-scan.yml hadolint (blocking) + Trivy image scan (advisory)
- codeql.yml CodeQL for Python and JS, main + weekly
Plus .github/dependabot.yml (pip/npm/actions/docker), .github/CODEOWNERS,
and docs/security-ci.md explaining each check and the one-time settings.
All additive: no existing files are modified. Actions are pinned to commit
SHAs, tokens default-deny (permissions: {}), advisory scans never block,
and SARIF upload is gated to push so fork PRs do not fail on a read-only
token. Composes with the correctness CI in odysseus-dev#1015.
|
Quick status now that #3498 is in: I've rebased this onto the latest If someone picks this up to merge: it's all workflow files, so it'll need to go through the web UI (the API route trips over the missing |
alteixeira20
left a comment
There was a problem hiding this comment.
The security-gate direction and blocking/advisory split look appropriate for #2288, and the action pinning looks solid.
I cannot approve the current revision yet because it is 115 commits behind dev, so the June 8 green checks no longer validate the current tree. Please rebase and rerun the complete suite.
Please also reconcile the documented triggers. docs/security-ci.md says pushes to both dev and main are scanned, while the proposed workflows currently target pushes to main only. Since normal development lands on dev, either add dev coverage or update the documented policy explicitly.
One more point before this becomes merge-ready: please confirm that rebuilding the complete image with advisory Trivy on every PR is intentional, including documentation-only PRs. If not, consider limiting the PR job to relevant paths or separating push-only SARIF upload from PR scanning so PR jobs do not request write permissions unnecessarily.
After those points are resolved and the rebased checks pass, this should be suitable for approval.
Address review on odysseus-dev#1314 (points 2 and 3). container-scan.yml now runs only hadolint (the blocking Dockerfile lint) and keeps the broad pull_request + push:[main] trigger so the required check always reports and never hangs a PR. The advisory image scan moves to container-trivy.yml, split by event: - pull_request / workflow_dispatch: build and scan under contents:read only, no SARIF upload. The image build runs PR-supplied Dockerfile instructions, so this path holds no write scope. - push to main: build, scan, and upload SARIF with security-events:write. Only this trusted path is granted write. This stops PR jobs from requesting security-events:write they never use, and a paths-ignore (matching docker-publish.yml) skips the image rebuild on docs-only changes. docs/security-ci.md: correct the trigger description to "every pull request and every push to main", matching the workflows and the existing ci.yml convention. Verified locally: zizmor --offline --min-severity=low and actionlint are clean on the changed and new workflow files.
|
Pushed 4139cbf addressing points 2 and 3. Trigger reconciliation (point 2). The mismatch was in the doc, not the workflows. The Trivy (point 3). It was not intentional to rebuild on every PR or to carry write scope on PR runs. The advisory scan now lives in its own file,
A Rebase (point 1). The June 8 checks are stale; |
alteixeira20
left a comment
There was a problem hiding this comment.
LGFM.
Final validation passed on the current head. This is an additive security CI/governance baseline: exactly nine new files, no app-code changes, CI green, docs updated, workflow actions pinned, no unsafe trigger introduced by this PR, and Trivy correctly keeps PR scans read-only while reserving SARIF upload for pushes to main.
Pin actions to commit SHAs, set persist-credentials: false on every checkout, and scope token permissions to the jobs that use them. Suppress the two findings that are safe by design: the description bot's pull_request_target trigger (no fork code runs) and an intentional word-split in the docker manifest step. Clears actionlint and zizmor against dev so the blocking gate from odysseus-dev#1314 can pass once both land.
…#305-310) (odysseus-dev#1314) * ci: add security scanning suite and governance Consolidates the security CI work into one reviewable change. Adds, as separate workflow files under .github/workflows/: - secret-scan.yml gitleaks (pinned + checksum-verified), full history - workflow-security.yml actionlint + zizmor, audits the workflows themselves - dependency-review.yml PR dependency gate + advisory pip-audit - container-scan.yml hadolint (blocking) + Trivy image scan (advisory) - codeql.yml CodeQL for Python and JS, main + weekly Plus .github/dependabot.yml (pip/npm/actions/docker), .github/CODEOWNERS, and docs/security-ci.md explaining each check and the one-time settings. All additive: no existing files are modified. Actions are pinned to commit SHAs, tokens default-deny (permissions: {}), advisory scans never block, and SARIF upload is gated to push so fork PRs do not fail on a read-only token. Composes with the correctness CI in odysseus-dev#1015. * ci(security): isolate Trivy from the Dockerfile lint gate Address review on odysseus-dev#1314 (points 2 and 3). container-scan.yml now runs only hadolint (the blocking Dockerfile lint) and keeps the broad pull_request + push:[main] trigger so the required check always reports and never hangs a PR. The advisory image scan moves to container-trivy.yml, split by event: - pull_request / workflow_dispatch: build and scan under contents:read only, no SARIF upload. The image build runs PR-supplied Dockerfile instructions, so this path holds no write scope. - push to main: build, scan, and upload SARIF with security-events:write. Only this trusted path is granted write. This stops PR jobs from requesting security-events:write they never use, and a paths-ignore (matching docker-publish.yml) skips the image rebuild on docs-only changes. docs/security-ci.md: correct the trigger description to "every pull request and every push to main", matching the workflows and the existing ci.yml convention. Verified locally: zizmor --offline --min-severity=low and actionlint are clean on the changed and new workflow files. --------- Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
Summary
Adds a security-scanning baseline as nine new files under
.github/anddocs/, consolidating the six earlier per-scanner PRs (#305-310) into one reviewable change. Nothing existing is modified. Each scanner is its own workflow with a documented blocking-vs-advisory split, so the gate can be required incrementally rather than all at once. This is the security half of the CI baseline agreed in #2288.Target branch
dev, notmain.Linked Issue
Part of #2288
Type of Change
Checklist
dev.How to Test
These are CI workflow files, so the scanners are the verification, not the app.
devtree, the suite's own five workflows are clean:actionlint .github/workflows/{secret-scan,workflow-security,dependency-review,container-scan,container-trivy,codeql}.ymlreports nothing.zizmor --offline --min-severity=lowon those files reports no findings.hadolint --ignore DL3008 Dockerfileis clean (the suite ignores DL3008 on purpose; see below).docs/security-ci.mdlists every check and the one-time settings (branch protection plus the Security-tab toggles) needed to enforce them after merge.What is included
secret-scan.ymlworkflow-security.ymldependency-review.ymlcontainer-scan.ymlcontainer-trivy.ymlmainpushescodeql.ymlPlus
.github/dependabot.yml(pip / npm / actions / docker),.github/CODEOWNERS, anddocs/security-ci.md.Notes:
permissions: {}), each job opts back into read-only.pushso fork PRs don't fail on a read-only token.DL3008(unpinned apt versions) is intentionally ignored: a-slimbase purges old package versions from Debian's repos, so pinning breaks future rebuilds. The base image is pinned instead and tracked by Dependabot.Merging workflow files needs a token with
workflowscope, or a merge through the web UI.