Repository navigation
Harden updater container isolation - #667
Merged
Merged
Conversation
Run updater containers as the existing non-root user while dropping all Linux capabilities, enabling no-new-privileges, and explicitly selecting Docker's built-in seccomp profile. Reject Docker hosts that do not advertise seccomp support, including correct handling of modern and legacy API security-option formats. Keep local staging capability-free by assigning the updater UID and GID through moby/go-archive, and run required certificate, repository, and case-insensitive setup commands with checked exit handling. Preserve existing repository configuration, CIFS behavior, internal-only updater networking, proxy credential separation, and main updater exit semantics. Roll back case-insensitive storage containers and volumes on initialization failures with bounded cleanup and explicit ownership transfer to Updater on success. Cover container requests, minimum Docker API compatibility, setup failures, cleanup paths, archive ownership, and effective runtime controls for normal, local, case-insensitive, and combined flows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090f48f7-de21-4eb1-b076-8c5852c16f14
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The security controls and cleanup paths are well covered; only a non-blocking fixture spelling correction was noted.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Hardens updater container isolation and makes setup failures fail closed while preserving local and case-insensitive workflows.
Changes:
- Drops all updater capabilities and explicitly enables seccomp and no-new-privileges.
- Adds checked setup execution, capability-free local staging, and bounded storage rollback.
- Expands unit and runtime security coverage.
| File | Description |
|---|---|
internal/infra/updater.go |
Adds container hardening, seccomp checks, checked execution, and storage cleanup. |
internal/infra/updater_test.go |
Tests security configuration and cleanup ownership. |
internal/infra/run.go |
Adds checked setup and UID/GID-aware local staging. |
internal/infra/run_test.go |
Tests setup failure propagation and archive ownership. |
testdata/scripts/security.txt |
Verifies runtime security and writable local staging. |
testdata/scripts/smb-mount.txt |
Verifies case-insensitive runtime security. |
testdata/scripts/smb-mount-local.txt |
Covers combined local and case-insensitive operation. |
testdata/scripts/local.txt |
Tests clean and dirty local repository handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JamieMagee
approved these changes
Oct 6, 2026
jakecoffman
approved these changes
Oct 7, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 7, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 7, 2026
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.

Summary
Harden the untrusted Dependabot updater container while preserving the existing trust boundary: the updater remains non-root and internal-only, raw credentials remain in the trusted proxy, and the proxy remains the only component connected to both internal and internet-capable networks.
This is defense-in-depth hardening. It does not address a demonstrated credential disclosure.
Updater container security controls
internal/infra/updater.gonow constructs the updaterHostConfigwith:CapDrop: ALLCapAddentriesno-new-privileges=trueseccomp=builtinDropping every capability is compatible with the updater because package-manager and repository code runs as the existing
dependabotuser and does not need privileged kernel operations. No capability was added back.no-new-privilegesprevents updater processes from gaining privilege through setuid/setgid binaries or file capabilities. The explicit built-in seccomp selection preserves Docker's reviewed default syscall policy and never usesseccomp=unconfined.The updater remains attached only to the no-internet network. The proxy continues to bridge the internal and internet-capable networks and remains solely responsible for resolving and injecting credentials into outbound requests.
Fail-closed seccomp compatibility
Before creating any updater or case-insensitive storage resources, the CLI calls Docker Info and decodes
SecurityOptionswith the pinned Dockersystem.DecodeSecurityOptionshelper.This accepts both daemon representations that indicate seccomp support:
name=seccomp,profile=builtinseccompMalformed security options are returned with decoding context. If seccomp is genuinely absent, updater creation fails with
ErrSeccompUnavailable; there is no retry or fallback that removes the security option or runs the updater unconfined.Request-level tests use API 1.44 for the modern representation and the actual minimum API 1.24 for the legacy representation. The legacy test requires
/v1.24/infoand/v1.24/containers/create, so it verifies protocol compatibility rather than only supplying a legacy value to a modern client.Checked required setup commands
The main updater command intentionally records its exit code in
Updater.ExitCode, so its existingRunCmdbehavior is unchanged. Required setup operations now use a separate checked streaming execution path and stop the update immediately on any nonzero exit:update-ca-certificatesgit safe.directoryconfigurationThis prevents setup failures such as a missing
gitexecutable from being mistaken for successful preparation.Capability-free local staging
Local source files are no longer prepared by granting updater-wide ownership capabilities. The CLI:
dependabotuser;moby/go-archiveTarWithOptionsAPI;ChownOpts; andThis makes the staged files writable by the updater without
CAP_CHOWN,CAP_DAC_READ_SEARCH, a rootchown, or any persistentCapAddentry.Local Git setup preserves an existing repository instead of reinitializing it, which retains settings such as Windows
core.filemode=false. A clean existing repository remains on its original commit. Dirty or non-repository sources receive the Dependabot CLI automated commit so updater changes are not lost. Unexpected Git failures still propagate; only the expected clean staged-diff result is treated as success.Case-insensitive filesystem compatibility and cleanup
The CIFS mounts retain
noperm, allowing the non-root updater to access files whose ownership is presented by CIFS rather than granting Linux capabilities. The updater configures/dpdbot/repoas a Git safe directory only when the case-insensitive experiment is enabled.Case-insensitive storage initialization now has explicit resource ownership and rollback:
Updater.Close, which removes the updater, storage volumes, and storage container at the end of the run.This avoids leaking case-insensitive storage resources on partial initialization while preventing double cleanup after ownership transfer.
Tests and fixtures
Request and unit coverage
internal/infra/updater_test.gonow verifies:ContainerCreateuses the non-root user;no-new-privileges=trueandseccomp=builtinare explicit and never unconfined;ContainerCreate;noperm;Updater.Close.internal/infra/run_test.goverifies:Runtime/script coverage
The runtime Dockerfiles use portable
COPYplusRUN chmodinstructions so the repository's restricted script harness can use Docker's legacy builder without requiring BuildKit.The focused scripts verify:
CapEff=0,NoNewPrivs=1,Seccomp=2;--localplus case-insensitive execution: the same controls, successful local staging onto/dpdbot/repo, and a real Git operation without a dubious-ownership error;git apply, avoiding shell-redirection differences across platforms.Validation
Validated from commit
cf22640on October 6, 2026:go test -count=1 ./internal/infrasecurity,local,smb-mount, andsmb-mount-localscript tests withDOCKER_BUILDKITand custom Docker configuration removedgo test -run '^$' ./...go vet ./...go mod tidy -diffgofmt -l .git diff --check HEAD^..HEADThe focused runtime tests reached and passed all assertions for
CapEff=0,NoNewPrivs=1, andSeccomp=2in normal, local, case-insensitive, and combined local-plus-case-insensitive flows.Representative update tests against
brettfo/db-testalso passed from the same HEAD:/npm-version-range:pad-left2.0.1 -> 2.1.0use_case_insensitive_filesystem, directory/casing:Newtonsoft.Json13.0.1 -> 13.0.4Both output files contained
create_pull_request, and neither contained the raw GitHub access token.Compatibility and rollout
Docker hosts must advertise seccomp through either the modern or legacy Docker Info security-option format. Hosts without kernel/daemon seccomp support now fail before updater creation instead of silently running unconfined.
Downstream Azure DevOps integrations must use the first released Dependabot CLI version that contains this change. That release version has not yet been assigned.