feat(vuln-scanner): add ZAP + Nuclei DAST scanning of MIT Learn QA - #5632
Open
shaidar wants to merge 5 commits into
Open
feat(vuln-scanner): add ZAP + Nuclei DAST scanning of MIT Learn QA#5632shaidar wants to merge 5 commits into
shaidar wants to merge 5 commits into
Conversation
Adds a new vuln_scanner Pulumi project deploying OWASP ZAP and Nuclei as weekly Kubernetes CronJobs against MIT Learn's QA endpoint, on the operations cluster. Findings are uploaded to S3 and imported into AWS Security Hub via ASFF, with a diff-and-archive step so fixed findings actually clear instead of sitting ACTIVE forever, and CVE-specific typing for Nuclei matches that reference a known CVE. No existing AWS-native tooling (Security Hub/Inspector/GuardDuty, all confirmed enabled in this account) tests live application behavior -- this closes that specific gap rather than duplicating what's already running. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… reporter
The reporter's Dockerfile was single-stage FROM mitodl/ol-python-base,
which has a shell and a package manager -- it didn't actually match
gwarek's stated runtime posture ("ships no shell") the way the code
comments claimed.
Switched to a multi-stage build: ol-python-base for the build stage,
gcr.io/distroless/base-debian13 (matching Debian release, verified by
hand -- a mismatched pair fails at container start with a glibc
symbol-version error, not a build-time error) for a genuinely shell-less
final stage, with the built CPython interpreter and venv copied across.
Verified end-to-end with real docker build/run: boto3 imports and
reporter.py's own entrypoint both work, /bin/sh is confirmed absent, and
the image shrank from 839MB to 147MB.
Also bumped to Python 3.14 (matching the newest precedent already in
this repo for standalone tool images -- release_bot and
kubewatch_webhook_handler both use python:3.14-slim) rather than 3.12,
which nothing here actually requires.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pinned ghcr.io/zaproxy/zaproxy and projectdiscovery/nuclei to real digests captured via `docker buildx imagetools inspect`, replacing the floating :stable/:latest tags. While verifying Nuclei's CLI against the actual pinned image (v3.11.1), found that -td was never a real flag -- it would have failed at container start. Also found that -t/-update-template-dir don't compose the way pointing both at the same custom directory suggests: doing that made Nuclei decide templates "weren't installed" there and auto-install a second, nested copy, so the scan would have silently run a smaller, different template set than the one -update-templates had just written. Fixed by relying on $HOME alone (already set for this container) -- verified end-to-end with real `docker run`, including an actual scan against example.com: templates install to $HOME/nuclei-templates and the scan step finds them with no extra flags, no nested-install surprise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires vuln-scanner into the simple_pulumi meta-pipeline (the same mechanism release_bot uses for its own image build + deploy), so the reporter image gets built and pushed to ECR and the QA stack gets deployed automatically via a reviewed preview gate, rather than needing a one-off manual pipeline. Registered in three required places (each backed by a test that fails if forgotten): production_app_names in meta.py, PROJECT_VERSIONS in versions_map.py, and PROJECT_SECRETS in secrets_map.py (both empty -- this project reads no version-pin constants or SOPS secrets yet). topology="preview-gated", same reasoning as release-bot: don't auto-deploy a tool that runs active attack payloads and holds real S3-write/Security-Hub credentials on every push. The build job's CONTEXT is always the whole pulumi_project_path directory, not the Dockerfile's own directory (confirmed by reading _build_docker_image_job, not assumed) -- since the reporter's Dockerfile lives in a reporter/ subdirectory rather than at the project root like release_bot's, its COPY sources needed to change from bare filenames to reporter/-prefixed paths. Verified by actually running `python pipeline.py vuln-scanner` (confirms the generated CONTEXT/ DOCKERFILE values) and a real `docker build -f reporter/Dockerfile .` from the vuln_scanner directory (confirms the image still builds and runs correctly under that same context). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ran pulumi stack init --secrets-provider=awskms://alias/infrastructure-secrets-qa QA for real, against live credentials. Adds the real secretsprovider/ encryptedkey Pulumi's own config writer generated, merged into the existing config file rather than replacing it -- verified via diff before committing that only those two keys were added. Also ran `pulumi preview` for real against this stack and against the operations.QA EKS stack (to check the eks:namespaces addition from an earlier commit): vuln_scanner's own preview now fails cleanly on "the vuln-scanner namespace doesn't exist yet" rather than any code error, confirming the Pulumi program itself is correct. Applying that namespace (a `pulumi up` on the EKS stack) is being held off since that stack's preview also surfaces two unrelated pending changes already drifted from main (a Traefik chart version bump, an AWS provider region-field rename) that would apply alongside it -- a deployment sequencing decision, not a code change, so left for a separate `up`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds weekly ZAP and Nuclei DAST scanning for MIT Learn QA, with S3 report storage and Security Hub finding synchronization.
Changes:
- Adds the vulnerability-scanner Pulumi application and Kubernetes CronJobs.
- Adds an ASFF reporter image and AWS finding lifecycle logic.
- Integrates deployment into the simple Pulumi pipeline.
Reviewed changes
Copilot reviewed 14 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds lint exceptions for scanner builders. |
src/ol_infrastructure/lib/ol_types.py |
Registers the vulnerability-scanner service. |
src/ol_infrastructure/infrastructure/aws/eks/Pulumi.operations.QA.yaml |
Adds the scanner namespace. |
src/ol_infrastructure/applications/vuln_scanner/__init__.py |
Adds the application package. |
src/ol_infrastructure/applications/vuln_scanner/__main__.py |
Defines scanner infrastructure and CronJobs. |
src/ol_infrastructure/applications/vuln_scanner/Pulumi.yaml |
Defines the Pulumi project. |
src/ol_infrastructure/applications/vuln_scanner/Pulumi.QA.yaml |
Configures the QA target and stack encryption. |
src/ol_infrastructure/applications/vuln_scanner/vuln_scanner_policy.hcl |
Adds the placeholder Vault policy. |
src/ol_infrastructure/applications/vuln_scanner/reporter/__init__.py |
Adds the reporter package. |
src/ol_infrastructure/applications/vuln_scanner/reporter/Dockerfile |
Builds the distroless reporter image. |
src/ol_infrastructure/applications/vuln_scanner/reporter/pyproject.toml |
Declares reporter dependencies. |
src/ol_infrastructure/applications/vuln_scanner/reporter/uv.lock |
Locks reporter dependencies. |
src/ol_infrastructure/applications/vuln_scanner/reporter/reporter.py |
Converts and synchronizes scan findings. |
src/ol_concourse/pipelines/versions_map.py |
Registers scanner version dependencies. |
src/ol_concourse/pipelines/secrets_map.py |
Registers scanner secret dependencies. |
src/ol_concourse/pipelines/infrastructure/simple_pulumi/pipeline.py |
Adds the build and gated deployment pipeline. |
src/ol_concourse/pipelines/infrastructure/simple_pulumi/meta.py |
Registers the generated pipeline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # No shell to chain commands through, and no CMD -- ENTRYPOINT calls the | ||
| # venv's own interpreter directly, mirroring the same constraint gwarek's | ||
| # migration job already works around (`__main__.py:723-726`). | ||
| ENTRYPOINT ["/opt/venv/bin/python3", "reporter.py"] |
Comment on lines
+350
to
+354
| f"nuclei -update-templates; " | ||
| f"nuclei -target {target_url} " | ||
| f"-jsonl -output {NUCLEI_REPORT_PATH}; " | ||
| f'echo "nuclei exited $?"; ' | ||
| f"test -f {NUCLEI_REPORT_PATH}" |
Comment on lines
+399
to
+403
| for i in range(0, len(identifiers), 100): | ||
| securityhub_client.batch_update_findings( | ||
| FindingIdentifiers=identifiers[i : i + 100], | ||
| RecordState="ARCHIVED", | ||
| ) |
| application_labels = {**k8s_global_labels.model_dump(), "app": "vuln-scanner"} | ||
|
|
||
| targets = vuln_scanner_config.require_object("targets") | ||
| zap_scan_type = vuln_scanner_config.get("zap_scan_type") or "baseline" |
Comment on lines
+351
to
+352
| f"nuclei -target {target_url} " | ||
| f"-jsonl -output {NUCLEI_REPORT_PATH}; " |
Comment on lines
+2
to
+7
| # NOTE: secretsprovider/encryptedkey are intentionally omitted here -- they | ||
| # are minted by a real `pulumi stack init QA --secrets-provider=awskms://...` | ||
| # run against live AWS/Pulumi credentials, which this authoring session does | ||
| # not have. Whoever runs that stack init for the first time should end up | ||
| # with those two keys added here by the Pulumi CLI itself; do not hand-write | ||
| # a fake encryptedkey value. |
| return existing | ||
|
|
||
|
|
||
| def archive_stale_findings( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
N/A
Description (What does it do?)
Adds a new
vuln_scannerPulumi project (src/ol_infrastructure/applications/vuln_scanner/) that runs OWASP ZAP and Nuclei as weekly Kubernetes CronJobs against MIT Learn's QA endpoint (api.rc.learn.mit.edu), on theoperationscluster's QA stack.Why these two tools and this scope: this AWS account already runs Security Hub (CIS + AWS Foundational Security Best Practices standards), Amazon Inspector (ECR image CVE scanning), and GuardDuty (threat detection) -- confirmed via live AWS CLI checks before any of this was built, specifically to avoid duplicating existing coverage. None of that tests live application behavior (XSS, SQLi, auth/business-logic bugs), which is the specific gap this closes. OpenVAS and a broader Trivy rollout were considered and intentionally dropped for the same reason (rationale documented inline in
__main__.py).Key design points:
openapijob against MIT Learn's confirmed OpenAPI schema (/api/v1/schema/) for endpoint discovery, since the default spider finds almost nothing against a bare JSON API.BatchImportFindings, so results land in the same triage surface as existing GuardDuty/Inspector/Config findings instead of a report nobody checks.BatchUpdateFindings) clears findings that no longer reproduce -- ASFF findings never auto-resolve on their own..../Vulnerabilities/CVEASFF type and populate theVulnerabilitiesfield, so they show up in Security Hub's CVE-specific views.baselinescan (no active attack payloads) until a first run is reviewed; flips tofullvia one config value.Screenshots (if appropriate):
N/A
How can this be tested?
pulumi previewon the newvuln_scannerproject (requires a realpulumi stack init QA --secrets-provider=awskms://alias/infrastructure-secrets-qafirst -- not yet done, see checklist).pulumi up:kubectl get cronjobs -n vuln-scanner.kubectl create job --from=cronjob/zap-mitlearn-qa zap-manual-test -n vuln-scanner(and thenuclei-mitlearn-qaequivalent), thenkubectl logsthe job.ol-vuln-scanner-reports-qaS3 bucket, and confirm the findings appear in Security Hub:aws securityhub get-findings --filters '{"GeneratorId":[{"Value":"zap-automation-framework/mitlearn-qa","Comparison":"EQUALS"}]}'Ids in Security Hub rather than duplicating them; fix/suppress one and confirm a third run archives it.Succeeded(notFailed) viakubectl get jobs -n vuln-scanner-- validates the initContainer exit-code handling.Locally validated: the full repo pre-commit suite (ruff, mypy, secret detection, yamllint, hadolint) passes on this branch. The reporter's ASFF-building logic was also exercised directly against realistic ZAP/Nuclei report fixtures (not just read through) to confirm the exact
BatchImportFindingspayload shape, cross-checked against AWS's own ASFF/BatchImportFindings documentation.Additional Context
This went through multiple rounds of review (automated code review plus manual verification against AWS's ASFF/BatchImportFindings documentation) that caught and fixed several real bugs before this PR, including: an unbounded cron schedule that would overflow past 6 targets, third-party ZAP/Nuclei images relying on unverified non-root defaults, an initContainer exit-code design that could silently drop reports when ZAP found real vulnerabilities, an archive-on-zero-findings bug that could wipe out previously-tracked findings on a silent scan failure, and a
CreatedAtbug that would have reset a finding's age on every weekly re-scan.Opened as a draft since the checklist below has real pre-enable items outstanding.
Checklist:
Run
pulumi stack init QAfor real -- done, realsecretsprovider/encryptedkeyare in this PR nowPin
ghcr.io/zaproxy/zaproxyandprojectdiscovery/nucleito specific digests before enabling the CronJob schedulesFly the updated
simple-pulumi-metapipeline (cd src/ol_concourse/pipelines/infrastructure/simple_pulumi/ && python meta.py && fly -t pr-inf sp -p simple-pulumi-meta -c definition.json) so the newpulumi-vuln-scannerpipeline (image build + QA deploy, preview-gated) actually gets created -- the config is in this PR and verified (python pipeline.py vuln-scannergenerates correctly, and the Dockerfile builds under the exact CONTEXT/DOCKERFILE paths Concourse will use), but nothing runs until this is fly'dVerify Nuclei's CLI flags against the pinned version -- found
-tdwas never a real flag and would have failed at container start; fixed (see commits)Confirm with whoever owns MIT Learn QA's APISIX route config whether
chaitin-waf/rate-limiting plugins are active, and coordinate before flippingzap_scan_typetofullNew finding from a real
pulumi previewonoperations.QA(the EKS stack this PR adds thevuln-scannernamespace to): that stack has two pending changes already drifted frommain, unrelated to this PR -- a Traefik chart version bump and an AWS provider region-field rename. Applying the namespace requires apulumi upon that shared stack, which would apply those too. Decide whether to apply everything together or land the Traefik/provider drift separately first, then runpulumi uponoperations.QAbefore this PR'''s ownpulumi upcan succeed (the namespace has to actually exist first).