Skip to content

Commit 7a41edc

Browse files
authored
Merge pull request #91 from CodesWhat/fix/release-image-build-args
🔒 v1.3 GA hardening: review fixes, multi-lens sweep, release-image metadata
2 parents dce6283 + ac5731f commit 7a41edc

34 files changed

Lines changed: 771 additions & 41 deletions

.github/workflows/release-from-tag.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ jobs:
214214
# latest on stable tags only
215215
type=raw,value=latest,enable=${{ startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-') }}
216216
217+
- name: Compute build date
218+
id: build_date
219+
run: echo "date=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$GITHUB_OUTPUT"
220+
217221
- name: Build and push
218222
id: build
219223
continue-on-error: true # allow manifest retry path on transient push failures
@@ -224,7 +228,10 @@ jobs:
224228
platforms: ${{ env.DOCKER_PLATFORMS }}
225229
tags: ${{ steps.meta.outputs.tags }}
226230
labels: ${{ steps.meta.outputs.labels }}
227-
build-args: VERSION=${{ steps.meta.outputs.version || github.ref_name }}
231+
build-args: |
232+
VERSION=${{ steps.meta.outputs.version || github.ref_name }}
233+
COMMIT=${{ github.sha }}
234+
BUILD_DATE=${{ steps.build_date.outputs.date }}
228235
sbom: true
229236
cache-from: type=gha
230237
cache-to: type=gha,mode=max

CHANGELOG.md

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Security
11+
12+
- **Zero-padded numeric UIDs can no longer bypass root-user enforcement.** `require_non_root_user` (container create) and `allow_root_user: false` (exec) compared the user field to the exact string `"0"`, but Docker parses `Config.User` numerically — so `"00"`, `"000"`, or `"0000:5"` all run as root while slipping past the check. Both inspectors now parse the UID and treat any value resolving to 0 as root.
13+
- **A wide-open dedicated admin listener is now a validation error, not just a startup warning.** A non-loopback plaintext admin listener with no `clients.allowed_cidrs` exposes candidate-YAML submission and policy metadata with no authentication and no IP backstop. Validation now rejects that shape unless the operator sets `clients.allowed_cidrs`, configures `admin.listen.tls`, or explicitly acknowledges the exposure with the new `admin.listen.insecure_allow_wide_open: true`.
14+
15+
### Fixed
16+
17+
- **A hot reload that rotates `policy_bundle.signature_path` no longer wedges every subsequent reload.** Bundle verification resolved the signature file from the last-applied config instead of the candidate, so changing `signature_path` (which is intentionally reload-mutable) loaded the wrong bundle, failed the digest check, and — because the active config only advances on success — left the operator permanently stuck until a restart. Verification now reads the path from the candidate config.
18+
- **`SOCKGUARD_REQUEST_BODY_CONTAINER_CREATE_ALLOW_SYSCTLS` and the `image_trust.verify_timeout` env vars are no longer silently ignored.** Three `request_body` keys were missing their `SetDefault` registration, and Viper only unmarshals registered keys — so configuring those fields purely through their `SOCKGUARD_*` env vars (no config file) had no effect. All three are now registered.
19+
- **Oversized `POST /nodes/{id}/update` and `POST /build` request bodies now return `413` instead of `403`,** matching every other body-limited inspector (a policy `403` implied the request was forbidden by rule rather than too large).
20+
- **Release images now carry real `commit` and `built` metadata.** The release workflow's Docker build only passed the `VERSION` build arg, so `sockguard version` in published images reported `commit unknown / built unknown`. The workflow now passes `COMMIT` (the tagged SHA) and `BUILD_DATE` (UTC build timestamp) through to the existing ldflags wiring.
21+
- **The container-label ACL exclusivity warning now fires once per process instead of on every hot-reload.** It was wired into the handler-chain build, which reruns on each config reload, so long-running deployments with reload enabled accumulated duplicate warnings. The warning is now gated by a process-lifetime guard (and finally has test coverage for both the enable-check and the once semantics).
22+
- **Coalesced inspect-cache waiters now honor their own context.** A request that joined an in-flight upstream inspect blocked unconditionally until that inspect finished — a slow daemon could pin waiter goroutines past their own deadlines. Waiters now return `ctx.Err()` promptly on cancellation while the in-flight resolve (and its cache fill) continues for future callers.
23+
- **`dockerfilters.Decode` now rejects `filters` parameters over 64 KiB before JSON parsing.** Real clients encode a handful of short terms; the only previous bound was the 1 MiB header limit.
24+
25+
### Changed
26+
27+
- **Test coverage for review-flagged gaps:** legacy (map-format) Docker filter encoding through the ownership and visibility rewrites end-to-end, the visibility meta-cache's one-inspect-per-TTL-window behavior, admin path-normalization variants on the policy-version interceptor (the validate interceptor already had them), and double-encoded `%252f` admin paths pinning the fail-closed pass-through. Corrected the visibility meta-cache code comment and the 1.3.0-rc.2 changelog wording: the cache accelerates repeated single-resource reads — list responses are filtered from their own payload and never inspect upstream.
28+
- **Upstream inspect responses are now drained before close** on the ownership and visibility owner-isolation paths, so idle keep-alive connections to the daemon are reused instead of discarded after each inspect.
29+
- **Corrected misleading exec comments in the `cis-docker-benchmark` and `gitlab-runner` presets** that referenced a non-existent `allowed_users` field; exec identity is controlled by `allow_root_user` and the argv `allowed_commands` allowlist.
30+
1031
## [1.3.0-rc.2] - 2026-06-10
1132

1233
### Security
@@ -26,7 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2647

2748
### Performance
2849

29-
- **Name/image pattern visibility checks no longer cost one daemon round-trip per list item.** Resource-meta inspects (`name_patterns` / `image_patterns` axes) now go through the same TTL-LRU, request-coalescing cache as label lookups, so a monitoring tool polling `GET /containers/json` against 50 containers triggers at most one upstream inspect per resource per 10-second TTL window instead of 50 per poll.
50+
- **Name/image pattern visibility checks on single-resource reads no longer cost one daemon round-trip per request.** Resource-meta inspects (`name_patterns` / `image_patterns` axes on `GET /containers/{id}/json` and other single-resource paths) now go through the same TTL-LRU, request-coalescing cache as label lookups, so repeated polls of the same resource trigger at most one upstream inspect per 10-second TTL window. (List responses were never affected — they are filtered from their own payload without upstream inspects.)
3051
- **File logging uses a 64 KiB buffer (up from 4 KiB),** cutting flush frequency and writer-mutex hold time on busy hosts; the 1-second periodic flush still bounds record staleness on quiet ones.
3152

3253
## [1.2.0] - 2026-06-02

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,9 +461,9 @@ LinuxServer's socket-proxy env surface is already Tecnativa-compatible for the b
461461

462462
| Tier | Theme |
463463
|---|---|
464-
| Security hardening (v1.x) | Continued mutation-test hardening of the rule-evaluation core and config validators; swarm `ContainerSpec.User` / `Privileges` enforcement parity with container create; wide-open admin listener promoted from startup warning to error behind an explicit opt-in flag |
464+
| Security hardening (v1.x) | Continued mutation-test hardening of the rule-evaluation core and config validators; swarm `ContainerSpec.User` / `Privileges` enforcement parity with container create; `HostConfig.SecurityOpt` `label=`/`systempaths=` policy evaluation (currently passed through) |
465465
| Policy refinement (v1.x) | Multiple frontend listeners on the main proxy, named rule path aliases |
466-
| Internals (v1.x) | Code-review backlog: collapse the config → filter-options → policy translation layers behind a single source of truth (generated Viper defaults); allocation-free rate-limit bucket state (packed `atomic.Uint64`); profiling-gated JSON redaction fast path |
466+
| Internals (v1.x) | Code-review backlog: collapse the config → filter-options → policy translation layers behind a single source of truth (generated Viper defaults); allocation-free rate-limit bucket state (packed `atomic.Uint64`); profiling-gated JSON redaction fast path; clear per-profile in-flight gauges when a hot reload removes the profile |
467467
| Compliance (v1.x) | CIS Docker Benchmark control mapping, audit-ready policy templates |
468468
| Multi-host (v1.3) | Remote Docker TCP upstreams, multi-upstream fan-out, remote daemon health checking, connection pooling, automatic failover |
469469
| Extensibility (v1.x+) | Optional plugin extension points (WASM or Go plugins), OPA/Rego policy integration |

app/configs/cis-docker-benchmark.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ request_body:
115115
# 5.22 / 5.23 — no privileged exec, no root exec).
116116
exec:
117117
allow_privileged: false
118-
# allowed_users left empty = no `User:` override on exec. Combine
119-
# with allow_root_user: false below for full CIS 5.23 enforcement.
118+
# allow_root_user: false denies any exec whose User resolves to root
119+
# (empty, "root", or numeric 0 incl. zero-padded). For full CIS 5.23
120+
# enforcement, also pin the permitted argv with allowed_commands.
120121
allow_root_user: false
121122

122123
# Read responses: redact host paths, env, network topology, and other

app/configs/gitlab-runner.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ request_body:
147147
# is still denied; the runner never requests it in normal operation.
148148
allow_privileged: false
149149
# Most official CI images (alpine, ubuntu, node, python) run job scripts
150-
# as root. Set allow_root_user: false and provide allowed_users if your
151-
# pipeline enforces a non-root image standard.
150+
# as root. Set allow_root_user: false if your pipeline enforces a non-root
151+
# image standard; pair with allowed_commands to pin the permitted argv.
152152
allow_root_user: true
153153

154154
image_pull:
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package admin
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// The policy-version interceptor reaches matchesAdminPath through
11+
// serviceUnavailableMiddleware and its own handler, so it needs the same
12+
// path-variant coverage the validate interceptor has — a normalization
13+
// regression in either wiring would otherwise leak variants to the rule
14+
// evaluator.
15+
func TestPolicyVersionInterceptorMatchesPathVariants(t *testing.T) {
16+
t.Parallel()
17+
for _, variant := range []string{
18+
testPolicyVersionPath + "/",
19+
"/admin/policy//version",
20+
"/admin/policy/./version",
21+
"/admin/policy/x/../version",
22+
} {
23+
t.Run(variant, func(t *testing.T) {
24+
t.Parallel()
25+
v := NewPolicyVersioner()
26+
v.Update(PolicySnapshot{Rules: 1, Source: "startup"})
27+
next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
28+
t.Fatalf("path variant %q leaked past the policy-version interceptor", variant)
29+
})
30+
handler := NewPolicyVersionInterceptor(PolicyVersionOptions{
31+
Path: testPolicyVersionPath,
32+
Source: v.Snapshot,
33+
})(next)
34+
35+
req := httptest.NewRequest(http.MethodGet, "/", nil)
36+
req.URL.Path = variant
37+
rec := httptest.NewRecorder()
38+
handler.ServeHTTP(rec, req)
39+
40+
if rec.Code != http.StatusOK {
41+
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
42+
}
43+
})
44+
}
45+
}
46+
47+
// A double-encoded slash (%252f → literal "%2f" in the decoded path) must NOT
48+
// match the admin path: path.Clean does not percent-decode, so the request
49+
// falls through to the rule evaluator where default-deny rejects it. This
50+
// pins the fail-closed behavior for both interceptors.
51+
func TestAdminInterceptorsIgnoreEncodedSlashVariants(t *testing.T) {
52+
t.Parallel()
53+
t.Run("validate", func(t *testing.T) {
54+
t.Parallel()
55+
passedThrough := false
56+
next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
57+
passedThrough = true
58+
w.WriteHeader(http.StatusForbidden)
59+
})
60+
handler := NewValidateInterceptor(Options{Path: testPath, Validate: newOKValidator()})(next)
61+
62+
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("rules: []"))
63+
req.URL.Path = "/admin%2fvalidate"
64+
rec := httptest.NewRecorder()
65+
handler.ServeHTTP(rec, req)
66+
67+
if !passedThrough {
68+
t.Fatal("encoded-slash path was handled by the admin layer, want pass-through to rule evaluator")
69+
}
70+
})
71+
t.Run("policy-version", func(t *testing.T) {
72+
t.Parallel()
73+
passedThrough := false
74+
next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
75+
passedThrough = true
76+
w.WriteHeader(http.StatusForbidden)
77+
})
78+
v := NewPolicyVersioner()
79+
v.Update(PolicySnapshot{Rules: 1, Source: "startup"})
80+
handler := NewPolicyVersionInterceptor(PolicyVersionOptions{
81+
Path: testPolicyVersionPath,
82+
Source: v.Snapshot,
83+
})(next)
84+
85+
req := httptest.NewRequest(http.MethodGet, "/", nil)
86+
req.URL.Path = "/admin/policy%2fversion"
87+
rec := httptest.NewRecorder()
88+
handler.ServeHTTP(rec, req)
89+
90+
if !passedThrough {
91+
t.Fatal("encoded-slash path was handled by the admin layer, want pass-through to rule evaluator")
92+
}
93+
})
94+
}

app/internal/cmd/serve.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"net/http"
1414
"os"
1515
"path/filepath"
16+
"sync"
1617
"syscall"
1718
"time"
1819

@@ -751,19 +752,34 @@ func withClientACL(cfg *config.Config, logger *slog.Logger) func(http.Handler) h
751752
return clientacl.Middleware(cfg.Upstream.Socket, logger, serveClientACLOptions(cfg))
752753
}
753754

755+
// labelACLWarnOnce gates warnIfLabelACLEnabled to a single emission per
756+
// process. The handler chain is rebuilt on every config hot-reload, so an
757+
// unguarded warning at the chain-build site would repeat on each reload.
758+
var labelACLWarnOnce sync.Once
759+
754760
// warnIfLabelACLEnabled reminds operators that container-label ACLs are only
755761
// trustworthy when sockguard is the exclusive path to the Docker socket: a
756762
// workload that can reach the raw socket can create a container carrying
757763
// arbitrary <label_prefix>* permission labels and self-grant access the
758764
// policy never approved. Sockguard cannot detect other socket consumers, so
759-
// the invariant is stated at chain-build time rather than enforced.
765+
// the invariant is stated rather than enforced — once per process, on the
766+
// first chain build (startup or hot-reload) that has the feature enabled.
760767
func warnIfLabelACLEnabled(cfg *config.Config, logger *slog.Logger) {
768+
warnLabelACLOnce(cfg, logger, &labelACLWarnOnce)
769+
}
770+
771+
// warnLabelACLOnce is the testable core of warnIfLabelACLEnabled: the Once is
772+
// injected so tests can verify both the enable-check and the once-per-process
773+
// gating without racing other tests for the package-level guard.
774+
func warnLabelACLOnce(cfg *config.Config, logger *slog.Logger, once *sync.Once) {
761775
if !cfg.Clients.ContainerLabels.Enabled {
762776
return
763777
}
764-
logger.Warn("container-label ACLs are enabled: label grants are only trustworthy if sockguard is the ONLY consumer of the Docker socket — any workload with raw socket access can self-grant permissions via labels",
765-
"label_prefix", cfg.Clients.ContainerLabels.LabelPrefix,
766-
)
778+
once.Do(func() {
779+
logger.Warn("container-label ACLs are enabled: label grants are only trustworthy if sockguard is the ONLY consumer of the Docker socket — any workload with raw socket access can self-grant permissions via labels",
780+
"label_prefix", cfg.Clients.ContainerLabels.LabelPrefix,
781+
)
782+
})
767783
}
768784

769785
// withAdminClientACL applies ONLY the client CIDR allowlist to the dedicated
@@ -1170,7 +1186,10 @@ func newAdminHTTPServer(handler http.Handler) *http.Server {
11701186
// for parsing and expose policy metadata — are reachable by any host that can
11711187
// route to the port, with neither authentication nor an IP backstop (#21). The
11721188
// CIDR guard in buildAdminHandlerChain closes the gap when clients.allowed_cidrs
1173-
// is set; this warning covers the remaining case where it is empty.
1189+
// is set. Config validation now rejects this shape outright unless
1190+
// admin.listen.insecure_allow_wide_open acknowledges it, so by the time this
1191+
// warning fires the operator has explicitly opted in — it keeps the exposure
1192+
// visible in the logs rather than gating startup.
11741193
func warnIfAdminListenerWideOpen(cfg *config.Config, logger *slog.Logger) {
11751194
listen := cfg.Admin.Listen
11761195
if listen.Address == "" || listen.TLS.Complete() {

app/internal/cmd/serve_policy_bundle_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,41 @@ func TestPolicyBundleReload_ParsesVerifiedBytesNotFile(t *testing.T) {
266266
}
267267
}
268268

269+
func TestPolicyBundleReload_ResolvesSignaturePathFromCandidate(t *testing.T) {
270+
// Regression: verifyBundle must load the bundle entity from the CANDIDATE
271+
// config's signature_path, not the last-applied config's. signature_path is
272+
// reload-mutable so an operator can rotate the bundle; reading it from
273+
// activeCfg loaded the wrong (old) bundle for the new YAML and — because
274+
// activeCfg only advances on success — wedged every subsequent reload.
275+
initial := policyBundleInitialConfig()
276+
initial.PolicyBundle.SignaturePath = "/old/sig.bundle.json"
277+
278+
verifier := &stubBundleVerifier{res: policybundle.VerifyResult{Signer: "keyed:x", DigestHex: "ab"}}
279+
f := newPolicyBundleFixture(t, initial, verifier)
280+
281+
// The candidate (parsed from the verified bytes) points at the rotated path.
282+
candidate := config.Defaults()
283+
candidate.PolicyBundle.Enabled = true
284+
candidate.PolicyBundle.SignaturePath = "/new/sig.bundle.json"
285+
candidate.PolicyBundle.AllowedSigningKeys = []config.PolicyBundleSigningKey{{PEM: "stub"}}
286+
f.loadCfg = &candidate
287+
288+
var gotPath string
289+
f.coordinator.deps.loadBundleEntity = func(p string) (verify.SignedEntity, error) {
290+
gotPath = p
291+
return &stubEntity{}, nil
292+
}
293+
294+
f.coordinator.reload()
295+
296+
if gotPath != "/new/sig.bundle.json" {
297+
t.Fatalf("loadBundleEntity path = %q, want the candidate's /new/sig.bundle.json (not active /old)", gotPath)
298+
}
299+
if got, ok := metricsReloadCount(t, f.registry, "ok"); !ok || got != 1 {
300+
t.Fatalf("ok count = %d (found=%v), want 1", got, ok)
301+
}
302+
}
303+
269304
// ---------------------------------------------------------------------------
270305
// verifyPolicyBundleAtStartup — exhaustive branch coverage.
271306
// Every failure branch of the signature gate aborts startup, so each one

app/internal/cmd/serve_reload.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"io"
78
"log/slog"
89
"path/filepath"
@@ -273,14 +274,28 @@ func (c *reloadCoordinator) verifyBundle() (*policybundle.VerifyResult, []byte,
273274
if c.bundleVerifier == nil || !c.activeCfg.PolicyBundle.Enabled {
274275
return nil, nil, nil
275276
}
276-
if c.activeCfg.PolicyBundle.SignaturePath == "" {
277-
return nil, nil, errors.New("policy_bundle.signature_path is empty")
278-
}
279277
yamlBytes, err := c.deps.readConfigBytes(c.cfgFile)
280278
if err != nil {
281279
return nil, nil, err
282280
}
283-
entity, err := c.deps.loadBundleEntity(c.activeCfg.PolicyBundle.SignaturePath)
281+
// Resolve the bundle path from the CANDIDATE config, not c.activeCfg.
282+
// signature_path is intentionally reload-mutable (reload/diff.go) so an
283+
// operator can rotate the bundle file; reading it from the last-applied
284+
// config would load the wrong (old) bundle for the new YAML, fail the
285+
// digest check, and — because activeCfg only advances on success — leave
286+
// every subsequent reload permanently stuck on the stale path until a
287+
// restart. The trust root that actually authenticates the bundle is
288+
// reload-immutable, so taking the path from the unverified candidate adds
289+
// no trust: a forged path still needs a bundle signed by the pinned key.
290+
candidate, err := c.deps.loadConfigBytes(yamlBytes)
291+
if err != nil {
292+
return nil, nil, fmt.Errorf("parse candidate config for signature_path: %w", err)
293+
}
294+
signaturePath := candidate.PolicyBundle.SignaturePath
295+
if signaturePath == "" {
296+
return nil, nil, errors.New("policy_bundle.signature_path is empty")
297+
}
298+
entity, err := c.deps.loadBundleEntity(signaturePath)
284299
if err != nil {
285300
return nil, nil, err
286301
}

0 commit comments

Comments
 (0)