Repository navigation
Conversation
…slim them CLI_VERSION was declared at the top of the Dockerfile. Every RUN after an ARG is cache-keyed on its value, and CLI_VERSION changes on every release, so each release rebuilt every toolchain layer even when nothing in them changed. Between 2.11.10 and 2.11.11, 1,028 MB of the 1,049 MB compressed amd64 image was new layers, so every upgrade was close to a full pull. The release ARGs are now declared just before the CLI install, and a unit test keeps them there. The image also drops payload nothing runs: - rustup's minimal profile replaces the default one, which added offline HTML docs, rustfmt and clippy. Reachability runs only cargo. - npm and pip caches are no longer left in the image. - strace is no longer installed. - A .dockerignore keeps tests and other repo files out of the build context. dist/ stays in, since Dockerfile.preview copies the preview wheel from it.
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 29a60c9. Configure here.
…e the image in CI The release build cached layers with type=gha. Actions caches are scoped to the ref that wrote them, and each release builds on its own tag, so no release could read another's cache: every release rebuilt every layer under a new digest, and each wrote about 2.6 GB of cache nobody read. A registry cache at socketdev/cli:buildcache is readable from any tag, and reusing its layers keeps their digests stable across releases. e2e-image.yml builds the image from the commit, reusing that cache read-only, and runs --reach inside it against the npm fixture and a new Rust fixture. It follows e2e-test.yml's retry policy for the backend's intermittent empty results. Validation asserts what the image controls: the run completes and Coana reaches a reachable or unreachable verdict for at least one vulnerability, which needs the ecosystem's toolchain to work. It does not assert which verdict, since that depends on Coana and the backend.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Oskar Haarklou Veileborg (BarrensZeppelin)
left a comment
Oskar Haarklou Veileborg (BarrensZeppelin)
left a comment
There was a problem hiding this comment.
[agent] Simplification pass (quality only, not a bug hunt). Five suggestions and one comment-wording note, all inline. Looked fine: the Dockerfile parser in the test, the validator's own logic, the minimal Rust profile with cargo --version, npm cache clean, dropping apk update, adding Dockerfile to the unit-test push paths, and the .dockerignore entries (none are dead).
| if args.probe: | ||
| return 0 if alert_states(args.fixture) else 1 |
There was a problem hiding this comment.
[agent] --probe repeats tests/e2e/reach-facts-probe.sh. This script could be the single implementation, with reach-facts-probe.sh reduced to uv run python tests/e2e/<script>.py <fixture> --probe. The script would then want a name that isn't specific to the image.
There was a problem hiding this comment.
Done in ede86cf. tests/e2e/reach_facts.py (renamed, so the name isn't image-specific) is the single implementation of the probe. reach-facts-probe.sh is gone: e2e-test.yml's retry-probe and validate-reachability.sh call uv run python tests/e2e/reach_facts.py <fixture> --probe directly, and the image job runs the same script with the image's Python.
The reachability engine sets a minimum Node version, and Dependabot doesn't know Node's release schedule, so it would propose odd-numbered majors, which never get LTS. The floating node tag already picks up minor and patch releases.
… the Dockerfile guards - tests/e2e/run-with-retries.sh is the one retry loop for the backend's intermittent empty results, used by e2e-test.yml and e2e-image.yml. - tests/e2e/reach_facts.py is the one probe and the one definition of the inconclusive backend result, replacing reach-facts-probe.sh and the image-only validator. validate-reachability.sh calls it. - e2e-image.yml builds the image once and runs every fixture against it, recording each fixture's result so one failure doesn't hide another. - The Dockerfile test lists the ARGs allowed above the toolchain layers and fails on any other, so a new per-release ARG is caught too. - PIP_NO_CACHE_DIR as a build ARG replaces the per-call --no-cache-dir flags. - .dockerignore lists what the build context includes instead of what it excludes.
…ayers # Conflicts: # CHANGELOG.md # pyproject.toml # socketsecurity/__init__.py # uv.lock
Every release currently rebuilds every layer of the Docker image, so upgrading
socketdev/clidownloads nearly the whole image. This PR fixes that and removes about 1.1 GB of payload nothing runs.Root cause of the per-release re-pull
ARG CLI_VERSION(with the other release ARGs) was declared at the top of the Dockerfile. BuildKit cache-keys everyRUNafter anARGon that ARG's value, andCLI_VERSIONchanges on every release, so every toolchain layer missed the cache on every release build even when nothing in it changed. From the published amd64 manifests:The base image digest was the same across 2.11.10 → 2.11.11, so the churn didn't come from upstream. The release ARGs are now declared just before the CLI install.
tests/unit/test_dockerfile.pylists the ARGs allowed above the toolchain layers (GO_VERSION,JAVA_VERSION,DOTNET_VERSION,TARGETARCH,PIP_NO_CACHE_DIR) and fails on any other, so a new per-release ARG is caught as well. It fails againstmain's Dockerfile.Dockerfileis added to the Unit Testspushpaths, since the suite now reads it.Release build cache
Moving the ARGs only helps if a release can read the previous release's cache, and it can't today.
release.ymlcaches withtype=gha. Actions caches are scoped to the ref that wrote them, and each release builds on its own tag: the repository's BuildKit cache entries all sit underrefs/tags/v2.11.x, a full set per release, none ever read by the next one. That is also why the Actions cache is pinned at its 10 GB limit. The release build now uses a registry cache atsocketdev/cli:buildcache(mode=max), which any tag can read.I checked the mechanism with a local registry and a multi-platform push. With the registry cache, two builds with different
CLI_VERSIONvalues produced identical layer digests for aRUNstep that writes a timestamp, so the step was reused rather than re-run. An inline cache (cache-to: type=inlinewithcache-frompointing at the published image) did not hit after a multi-platform push, which is why this uses a dedicated cache tag. The cache tag is pushed to the same Docker Hub repository and isn't meant to be pulled.Reachability inside the image (
e2e-image.yml)e2e-test.ymlruns reachability on the runner, so nothing checked that the image's toolchains actually work for Coana. The new workflow builds the image once from the commit (reading the release cache, never writing it) and runs--reachinside it against the existing npm fixture and a new Rust fixture (smallvec 1.6.0, which has a known advisory). Each fixture's result is recorded, so one failure doesn't hide the other. It runs when the Dockerfile, the reachability launcher, or the e2e fixtures change, on push tomain, and on demand. It isn't a required check.Both e2e workflows now share one policy for the backend's intermittent empty results:
tests/e2e/run-with-retries.shis the retry loop: up to three attempts while a probe fails, with retries surfaced as warnings.e2e-test.ymlnow calls it instead of its inline loop.tests/e2e/reach_facts.pyis the probe, and the one definition of the inconclusive backend result (a manifest uploaded, components orphaned, nothing to analyze). It replacesreach-facts-probe.sh, andvalidate-reachability.shcalls it instead of its owngreps.The image check asserts only what the image controls: the run completes, and Coana reaches a
reachableorunreachableverdict for at least one vulnerability, which needs the ecosystem's toolchain to work. It doesn't assert which verdict.Dependabot and Node majors
.github/dependabot.ymlnow ignores major updates to thenodeimage. Dependabot doesn't know Node's release schedule, so it proposes odd-numbered majors, which never become LTS, and it can't check the reachability engine's minimum Node version. The floatingnode:24-alpine3.24tag already picks up Node 24 minor and patch releases on every build. A Node major becomes a deliberate change, checked withe2e-image.yml.Payload removed
minimalprofile (rustc,cargo,rust-std) instead ofdefault. Drops about 900 MB of offline HTML docs, plusrustfmtandclippy. Reachability only invokescargo.npm cache clean --forceafter the global install, andARG PIP_NO_CACHE_DIR=1so nopip installleaves a cache. As an ARG it applies only at build time, and its value never changes, so it doesn't affect layer caching.apk updateis gone;apk add --no-cachedoesn't need it.straceis no longer installed (it was referenced nowhere). The changelog notesapk add stracefor anyone who relied on it..dockerignore: lists what the build context includes (package source, build metadata, entrypoint, anddist/forDockerfile.preview's wheel) instead of what it excludes, so new top-level directories stay out by default.Verification
CI's
dockerfile-smokejob builds this Dockerfile natively on amd64 and arm64 and runs its tool and TLS checks. The measurements below are from locallinux/arm64builds.socketdev/cli:2.11.11vs 3,446 MB for this branch (uncompressed, arm64)./root/.rustupwent from 1,693 to 753 MB,/root/.npmfrom 20 to 1 MB,/appfrom 11 to 2 MB.CLI_VERSION=2.11.10, then rebuilt withCLI_VERSION=2.11.11. Steps 2–17, everything through the locked dependency install, wereCACHED. Only the CLI install and the small layers after it rebuilt. A real release also changespyproject.toml/uv.lock, which adds the locked-dependency layer (about 21 MB). So an upgrade is roughly 25 MB uncompressed instead of about 1 GB compressed.socketcli,cargo/rustc,go,java,dotnet,node,ruby/bundle,pyenv,uvandsocketall run.cargo generate-lockfile,cargo metadataandcargo buildwork on a crate with a crates.io dependency.Rust reachability in the image is covered by
e2e-image.ymlon this PR. A throwaway run withnode:26-alpine3.24(#444, closed) also passed both fixtures, for when Node 26 is adopted.Once the registry cache is in use, the old
type=ghaBuildKit entries (about 10 GB) can be deleted from the repository's Actions caches.Bumps to
2.12.0, a minor version because the image drops binaries (strace,rustfmt,clippy) and changes how it is built and published. #438 also targets2.12.0; whichever merges second rides that version, and needs a small merge in the changelog and inrelease.yml's build step.Fixes CE-430
🤖 Generated with Claude Code
Note
Medium Risk
Changes the default Docker toolchain (minimal Rust, no strace) and layer layout that all CLI container users depend on; Rust reachability in the image was not fully re-verified in this PR.
Overview
Release 2.11.12 reshapes how the published
socketdev/cliimage is built so version bumps reuse cached toolchain layers instead of forcing a near-full re-download, and trims roughly a gigabyte of unused payload.Layer caching:
CLI_VERSIONand the other release-specific build args move to just before the CLI install step. BuildKit invalidates every subsequentRUNwhen those args change; having them at the top previously busted cache for Go, Java, Rust, pyenv, locked deps, etc. on every release. A new unit test intest_dockerfile.pyasserts release ARGs stay below toolchain installs, andDockerfileis included in the CI paths filter on push.Smaller image: Rust is installed with rustup’s minimal profile (no offline docs,
rustfmt, orclippy);straceis dropped from the base packages; npm and pip caches are cleared or use--no-cache-dir; redundantapk updateis removed. A new.dockerignorekeeps tests, docs, and similar out of the build context while still allowing preview wheels fromdist/.Reviewed by Cursor Bugbot for commit 29a60c9. Configure here.