Add wizcli security scan to bakery-build-native.yml - #715
Conversation
7813a0f to
a708ade
Compare
Scan-identifying metadata sent to
|
| Tag key | Source | Always present? | Example |
|---|---|---|---|
product |
image_target.image_name |
always | product=connect |
version |
image_target.image_version.name |
always | version=2025.09.0 |
channel |
image_target.release_channel.value |
always | channel=release |
platform |
SETTINGS.architecture |
always | platform=amd64 |
base-os |
tag template OS value |
only if the image has an OS axis | base-os=ubuntu-22.04 |
variant |
tag template Variant value |
only if the image has a Variant axis | variant=standard |
| (user-defined) | tool_options.tags (bakery.yaml) |
optional, appended last | team=platform |
os is a reserved tag key in Wiz, so the OS axis is emitted as base-os instead.
Context ID (--scan-context-id)
Groups all platform scans (amd64 + arm64) of the same build together and lets Wiz update a scan in place instead of creating a new one every run. Computed by default_scan_context_id, unless overridden by an explicit --scan-context-id:
- Non-release channels (
daily,preview):{image_name}-{channel}— one context per channel, always reflecting the latest build. - Release channel:
{image_name}-{version with patch/build-number stripped to monthly granularity}— patch bumps and build-number increments update the same context instead of creating a new one each time.
| Image | Channel | Version example | Context ID |
|---|---|---|---|
connect |
daily | — | connect-daily |
connect |
release | 2025.09.0 |
connect-2025-09 |
connect |
release (Positron build-number suffix) | 2026.08.1-2 |
connect-2026-08 |
connect-content |
daily | — | connect-content-daily |
connect-content |
release | r4.5-python3.14 |
connect-content-r4-5-python3-14 |
package-manager |
preview | — | package-manager-preview |
package-manager |
release | 2025.09.0 |
package-manager-2025-09 |
workbench |
preview | — | workbench-preview |
workbench |
release | 2025.09.0 |
workbench-2025-09 |
workbench-session |
release | r4.5-python3.14 |
workbench-session-r4-5-python3-14 |
Name (--name)
Not part of the grouping key (that's the context ID), but shown here since it's the other identifying label in the Wiz UI. Format: {image_name}:{Version}-{OS}-{Variant}-{platform}, matching bakery's per-platform cache tag format — so it maps directly to a real build artifact.
connect:2025.09.0-ubuntu-22.04-standard-amd64workbench:daily-ubuntu-22.04-standard-arm64
Scanning before Test resolved refs that were no longer addressable locally. Every target of an image builds to the same tagless temp name, so each build's `--load` overwrites the last and only one target stays reachable by name. `trivy image` falls back to a registry pull when a ref is not local, silently and successfully, because these images are published -- so the scan reported on the published image rather than the one the job just built. Test's `docker run` registers a `repo@sha256:...` entry per target, and unlike tags those coexist. Running after Test is what makes every target resolvable. #715 established this for the Wiz scan; the same constraint applies here and the comment now says so in both places. Renames the step to `Trivy Scan`. #715 adds its own step named `Scan` to this job, and two identically-named steps make every log line and annotation ambiguous. Wiz runs first of the two; its step slots between Test and Setup trivy when #715 lands.
Same placement bug the native workflow had. PR builds also use `--strategy build --pull --load`, so only the last target of an image stays addressable by tag, and this workflow passes no --metadata-file to fall back on digests. Scanning before Test resolved stale tags and silently scanned published images. Renames to `Trivy Scan` to match the native workflow, where it has to be distinct from #715's Wiz scan.
|
@rstu-js Could you please provide feedback on the name, context id, and tags? |
fba5b14 to
e128ce6
Compare
|
| context-id | scans that share it |
|---|---|
connect-2025-05 … connect-2025-11 |
4 each |
connect-2025-12 … connect-2026-04 |
8 each |
connect-2026-05 … connect-2026-07 |
12 each |
All 12 scans of 2026.07.0 send --scan-context-id connect-2026-07. They differ by
OS (3), variant (2), and platform (2).
Why this can be a problem
Wiz documents the flag as a baseline selector. The baseline is the last scan on the
same repository, branch, and context ID. Two results follow.
- The baseline is nondeterministic. Matrix jobs are
(image, version, platform), so
amd64 and arm64 run in parallel and write the same context. Job order decides which
scan becomes the baseline. - Cross-OS baselines can report false "resolved" findings. A CVE in 22.04 but not in
26.04 looks fixed. Nothing was fixed. I did not test this against a tenant.
Weekly rebuilds are the case that matters. That cron exists to pick up upstream
base-image changes.
Proposed change
One context ID per artifact. Keep the month-strip, so patch bumps continue a baseline:
--name connect:2026.07.0-ubuntu-22.04-std-amd64
--scan-context-id connect-2026-07-ubuntu-22-04-std-amd64
Tags keep the grouping. We already send product, version, channel, base-os,
variant, and platform.
Questions
- Does Wiz deduplicate findings inside a scan context, or is the context only a
baseline selector? This is the best argument to keep the collapse.std/minand
amd64/arm64overlap a lot. - Is there a limit on the length or character set of
--scan-context-id? The new IDs
reach about 55 characters. - Can a "resolved since baseline" signal exist that a cross-artifact baseline corrupts?
- Is coarse grouping deliberate practice?
If the answer to 1 is yes, I will revisit the variant and platform axes. OS stays
split, because finding overlap is lowest there.
version and the (currently coarse) scan-context-id both stay the same across rebuilds of the same release, so nothing in the existing tag set could answer which commit or which base image produced a given scan -- exactly the question weekly rebuilds raise per PR #715's context-id discussion. - BuildMetadata.revision reads the OCI revision label recorded in the build invocation. - BuildMetadata.base_image_digest(os_name) matches the OS material by name rather than assuming position, since multi-stage builds (e.g. the UV Python builder stage) record every stage's base image as a separate material. - ImageTarget.build_metadata_for_platform(platform) factors out the newest-first, platform-filtered lookup already used by ref(), so WizCLICommand.scan_tags can reach the same artifact ref() would resolve to. scan_tags emits revision=<sha> and base-digest=<digest> when build metadata is available, and omits them (not empty) otherwise, matching the existing os/variant convention.
|
At this point we have decided to keep |
rstu-js
left a comment
There was a problem hiding this comment.
Ben & I discussed removal of --scan-context-id since that is only supposed to be used in diff mode. Other than that, this plan & implementation looks good to move forward. Security will be doing a full threat model on the complete epic & implementation to address any best practice findings & security risks.
WizCLICommand now accepts policies/projects as plain CLI-passthrough fields, taking precedence over bakery.yaml's tool_options when set. - Needed because bakery.yaml has no env-var interpolation and the real wizcli binary has no native env var for either flag (confirmed via its own --help output), so CI has no other way to feed these from GitHub Actions secrets without writing them into bakery.yaml. Add --policies/--projects flags to bakery wizcli scan Threads the CLI-level override added in the previous commit through the plugin's execute() and WizCLISuite so 'bakery wizcli scan --policies ... --projects ...' works end to end. - Lets CI supply the Wiz policy/project IDs from secrets at scan time instead of writing them into bakery.yaml.
Scans every image built on a push/schedule/main-dispatch build with bakery wizcli scan, right after Build and before Test. Advisory only (continue-on-error: true) — a policy violation is visible in the log but never fails the build. - Gated on the existing push input plus a new wiz-auth presence check, so repos without WIZ_CLIENT_ID configured yet skip the step cleanly instead of showing a permanently failing one. - Auth, policy ID, and project ID all come from secrets declared here (WIZ_CLIENT_ID/WIZ_CLIENT_SECRET/WIZ_POLICY_ID/WIZ_PROJECT_ID); bakery.yaml is not involved.
Addresses three findings from the final whole-branch review: - `bakery wizcli scan` had no --dev-spec option, but the CI Scan step passed one unconditionally whenever a dev build was dispatched — reproducible as "Error: No such option: --dev-spec" on every matching build across all 3 product repos, silently swallowed by the step's continue-on-error. Now mirrors dgoss run's existing --dev-spec support exactly. - The Scan step resolved wizcli's path relative to the bakery context rather than $GITHUB_WORKSPACE, unlike Test's GOSS_PATH/DGOSS_PATH — safe only by coincidence of every caller leaving `context` at its default. Now pinned the same way. - setup-wizcli downloaded the wizcli binary with no integrity verification, unlike setup-goss/setup-hadolint. Wiz publishes an official GPG signature + SHA256 checksum for it; this wires up the verification chain from Wiz's own documented installation procedure.
wizcli scan unconditionally prepended "linux/" to the resolved platform string, so an already-prefixed value (e.g. "linux/arm64", exactly what bakery-build-native.yml's Scan step always passes via --image-platform "$IMAGE_PLATFORM") became "linux/linux/arm64" and matched zero targets. Reproduced live: a CI test run against a real Wiz tenant failed with "No image targets matching ... --image-platform ['linux/linux/arm64']" on every platform, in every repo. dgoss run hit this identical bug previously and already guards against it (test/plugins/builtin/dgoss/test_init.py:: TestDgossRunPlatformNormalization), so wizcli scan gets the same guard via a shared normalize_platform() helper in cli/common.py rather than a third copy-paste of the same three lines -- the deprecated `bakery run dgoss` bridge in cli/run.py already carried a second copy. - All three call sites (dgoss run, wizcli scan, and the deprecated run dgoss bridge) now call normalize_platform() instead of reimplementing the guard. - Adds a dedicated unit test for the helper and a regression test for wizcli scan; leaves each command's existing parametrized test in place as an integration check on the wiring.
The Filter Steps job step wrote each output with its own
echo ... >> "$GITHUB_OUTPUT" line. Adding the wiz-auth line for
wizcli support pushed the redirect count to three, which
shellcheck's actionlint hook flags as SC2129 (prefer a single
grouped redirect over repeated individual ones).
Group the three echoes into one { ...; } >> "$GITHUB_OUTPUT" block
to satisfy the lint rule.
version and the (currently unset) scan-context-id both stay the same across rebuilds of the same release, so nothing in the existing tag set could answer which base image produced a given scan -- exactly the question weekly rebuilds raise per PR #715's context-id discussion (tracked in #751). - BuildMetadata.base_image_digest(os_name) matches the OS material by name rather than assuming position, since multi-stage builds (e.g. the UV Python builder stage) record every stage's base image as a separate material. - ImageTarget.build_metadata_for_platform(platform) factors out the newest-first, platform-filtered lookup already used by ref(), so WizCLICommand.scan_tags can reach the same artifact ref() would resolve to. scan_tags emits base-digest=<digest> when build metadata is available, and omits it (not empty) otherwise, matching the existing os/variant convention. A commit-revision tag was considered too but dropped: Wiz's Commit Properties panel and Image Labels panel already surface the source commit independently, from CI context and the image's own OCI labels, so a tag would only duplicate it.
b1c2c11 to
786d8a9
Compare
- scan_name: {image}:{Version}-{OS}-{Variant}-{platform}, matching
bakery's per-platform cache tag format, so the Wiz UI name maps to a
real artifact.
- scan_tags: auto-emit product/version/channel/base-os/variant/platform
tags for grouping in the Wiz UI ("os" is a reserved tag key in Wiz,
hence "base-os"); user-supplied tool_options.tags are appended after.
Scan-context-id grouping is intentionally not addressed here; see
#751.
wizcli's mount and mountWithLayers drivers both require overlayfs or a containerd socket, neither of which is available on standard GitHub-hosted runners, and the scanned image is never present in the local Docker daemon since bakery pushes it by digest to a temp registry rather than loading it. extract (wizcli's own default) needs neither, so pin it explicitly rather than leave it implicit. Scans reference the image by digest only (repo@sha256:DIGEST, no :tag): when bakery builds multiple OS variants in one job, they all push to the same temp registry :latest tag sequentially, so by the time wizcli scans the second or third variant, :latest points to a different digest. Digest-only refs bypass tag resolution entirely. - Adds a `digest_only` keyword to ImageTarget.ref() so every caller can opt into a tag-free reference, replacing a wizcli-local duplicate of ref()'s metadata sort and platform-match loop. Logs at warning level when a reference falls back to a bare tag instead of doing so silently, since substituting a mutable tag for an exact digest changes which artifact a security scan reports on. - Adds coverage for digest_ref and ref(digest_only=True), neither of which had any tests.
The results table was built only from targets that produced a parseable report file, so a target whose scan errored vanished from it entirely. A run where five of six images failed to scan rendered a clean-looking one-row table, with the failures relegated to loose error text underneath. That misrepresents scan coverage in exactly the direction that matters for a security control. Failed targets now get a row with a "SCAN FAILED" verdict and "-" for each severity count. The counts are unknown rather than zero, so they are also excluded from the totals instead of quietly understating them.
wizcli's extract driver reads the local image store, but every target of an image builds to the same tagless temp name, so each build's --load overwrote the previous one. Only the last target stayed addressable, and the other five scans failed with "unknown image". Both mount and mountWithLayers were tried first and fail differently on hosted runners: no overlayfs graphdriver and no permission on the containerd socket. Moving the scan after Test fixes this without touching temp_name, whose taglessness push-by-digest requires. The docker run in Test resolves each target by digest -- Docker discards the tag portion when a digest is present -- which registers a name-free repo@sha256:... entry per target. Those coexist, unlike tags, so all targets are resolvable by the time the scan runs. Goss coverage is unaffected: it already resolved by digest, so it was testing the right image per target before and after the move. - Trade-off: a dgoss failure now skips the scan instead of preceding it.
`bakery wizcli scan` resolved `--image-platform` and then used it only to filter image targets, so the scan itself still fell back to the host architecture. On a cross-platform scan no build metadata matches the host platform, and ImageTarget.ref() silently degrades to a mutable registry tag, pointing wizcli at an artifact other than the one just built. The Wiz `--name` and `platform=` tag carried the host arch as well, mislabelling the results in the UI. Thread the resolved platform from the CLI command through WizCLIPlugin.execute, WizCLISuite, and WizCLICommand, defaulting to the host platform so existing callers are unaffected. Also raise the digest-to-tag fallback in ImageTarget.ref() from debug to warning. Substituting a mutable tag for an exact digest changes which artifact a security scan reports on, so it must be visible at default verbosity.
The installer fetched the binary, its checksum, the checksum's signature, and the verifying key all from downloads.wiz.io, then imported that key and verified against it. That proves only that the four downloads agree with each other -- anyone able to serve all four could supply a consistent set and pass. The fingerprint was recorded in a comment but never enforced. Require the signature to chain to the fingerprint Wiz publishes with its CLI install instructions, so the documented value is the trust anchor rather than either transport. Matching uses VALIDSIG's trailing primary-key field, since Wiz signs with a subkey. - Pin the full 160-bit fingerprint, not the 64-bit key ID, which is forgeable - Import into a throwaway GNUPGHOME so the runner's own keyring is left untouched; previously only the .asc file was removed - Verification now exits before chmod, so a rejected download cannot be left executable
When wizcli exited 0 but wrote no results file, or wrote one that could not be parsed, the target reached neither add_report nor add_failure. It disappeared from the results table while the log claimed the scan passed, which is the silent omission add_failure exists to prevent. wizcli is installed from an unpinned latest, so an output schema change is enough to trigger this. Recording now happens once per target outside the exit-code branch, so every target lands in the collection on every path. Exit 0 with no report gets its own "NO REPORT" verdict so the table distinguishes it from a scan that failed outright, and it appends a BakeryWizCLIError instead of logging a pass. CI runs the scan step with continue-on-error, so surfacing it does not block builds. The previously dead parse_err now travels into the error metadata, letting a parse failure explain itself in the rendered output rather than only in the log. Adds test_suite.py, which covers WizCLISuite.run() for the first time.
`+` survived both strip_patch and the trailing-digit strip, so it reached the final character substitution: `2026.07.0+build.5` became `connect-2026-07-build-5`. Every build of a `+meta` release therefore got its own Wiz context instead of updating one in place, which is the opposite of what the month-strip exists to do. Strengthen the surrounding tests, which passed for the wrong reasons: - The context-ID test asserted the emitted flag equalled the property it was built from, a tautology that held for any value. Replace it with a parametrization over real versions, one case per documented rule. The build-metadata case is the regression test for the fix above. - `scan_name` was compared against an expectation built with the same join expression as the implementation, so both could be wrong together. Assert a literal instead. - Tag assertions checked key presence only, which cannot see a wrong platform or OS value. Assert the exact tag list, and add a case for a target with no OS or variant so the two conditional branches are covered. Also correct two comments that described behavior the code does not have. The `--scan-context-id` comment claimed a uid-based, platform-agnostic default; the default is neither. The `base-os` comment cited a reserved key, but the real Wiz constraint is a three-character minimum on tag keys.
Three code-quality fixes deferred from the earlier review: - WizCLIDriverEnum constrains --driver to wizcli's actual accepted values (extract, mount, mountWithLayers) instead of a bare str, used consistently across the CLI option, WizCLICommand, and WizCLISuite. - WizScanReportCollection.table()'s count() closure now binds row as a default argument instead of capturing the loop variable by reference, fixing the late-binding closure hazard (ruff B023) even though it isn't currently enabled in this repo's lint config. - WizScanFailure replaces the bare str verdict used as a type-based sentinel for a failed scan in the same dict slot as a real WizScanReport. isinstance(report, str) is fragile; a dedicated type makes pass/fail a matter of type, not string content. test_suite.py assertions updated to expect WizScanFailure instead of raw strings.
version and the (currently unset) scan-context-id both stay the same across rebuilds of the same release, so nothing in the existing tag set could answer which base image produced a given scan -- exactly the question weekly rebuilds raise per PR #715's context-id discussion (tracked in #751). - BuildMetadata.base_image_digest(os_name) matches the OS material by name rather than assuming position, since multi-stage builds (e.g. the UV Python builder stage) record every stage's base image as a separate material. - ImageTarget.build_metadata_for_platform(platform) factors out the newest-first, platform-filtered lookup already used by ref(), so WizCLICommand.scan_tags can reach the same artifact ref() would resolve to. scan_tags emits base-digest=<digest> when build metadata is available, and omits it (not empty) otherwise, matching the existing os/variant convention. A commit-revision tag was considered too but dropped: Wiz's Commit Properties panel and Image Labels panel already surface the source commit independently, from CI context and the image's own OCI labels, so a tag would only duplicate it.
Drops --scan-context-id entirely -- the CLI passthrough, the scan_context_id field/plumbing through WizCLISuite and WizCLIPlugin, and the auto-generated default_scan_context_id -- rather than ship a grouping scheme that has not been reviewed. wizcli scans run without a scan-context-id for now, so Wiz falls back to its own default baseline selection. Design and implementation of a real scan-context-id strategy is tracked separately: #751
da0cb96 to
e2f354b
Compare
Same placement bug the native workflow had. PR builds also use `--strategy build --pull --load`, so only the last target of an image stays addressable by tag, and this workflow passes no --metadata-file to fall back on digests. Scanning before Test resolved stale tags and silently scanned published images. Renames to `Trivy Scan` to match the native workflow, where it has to be distinct from #715's Wiz scan.
Same placement bug the native workflow had. PR builds also use `--strategy build --pull --load`, so only the last target of an image stays addressable by tag, and this workflow passes no --metadata-file to fall back on digests. Scanning before Test resolved stale tags and silently scanned published images. Renames to `Trivy Scan` to match the native workflow, where it has to be distinct from #715's Wiz scan.
Adds
bakery wizcli scan(Wiz container vulnerability scanning) as a new advisory-only step in the sharedbakery-build-native.ymlworkflow, so every real build (push/schedule/main-dispatch) acrossimages-connect,images-workbench, andimages-package-managergets scanned once wired up downstream.Auth, policy ID, and project ID all come from four new optional secrets declared on this workflow (
WIZ_CLIENT_ID/WIZ_CLIENT_SECRET/WIZ_POLICY_ID/WIZ_PROJECT_ID) —bakery.yamlis never involved, and repos without the secrets configured skip the step cleanly rather than showing a permanently red one. The Scan step iscontinue-on-error: truethroughout: a policy violation is visible in the log but never blocks a build.This is the first of four coordinated PRs. The other three (
images-connect,images-workbench,images-package-manager) forward the new secrets to this workflow and can't merge until this one does — GitHub Actions fails workflow parsing if a caller passes a secret the reusable workflow doesn't declare. Populating the secret values themselves is a separate manual step per repo, not part of any of these PRs.Related:
wizcli#217