Conversation
Post-publication step from RELEASING.md's Helm section: docker buildx imagetools inspect codeswhat/sockguard:2.1.0 reports manifest-list digest sha256:65b3bc2a0f244aa784d6a24de04f4ed423d2d7be795f055fc6e25ef2aaa6ab2f, identical on ghcr.io, docker.io and quay.io. The next release metadata PR clears the pin again before its tag.
chore(chart): pin the 2.1.0 image to its multi-arch digest
v2.1.0 is out, so under the stated two-minor window 1.7.x leaves support. No 1.x maintenance branch remains.
… GO-2026-6355 golang.org/x/crypto v0.55.0 carries two denial-of-service bugs in the ssh package (CVE-2026-78662, CVE-2026-56855: a deadlocked channel stalls the connection), fixed upstream in v0.56.0. Sockguard never imports x/crypto/ssh; the module is an indirect requirement reached through sigstore-go and certificate-transparency-go for cryptobyte on the opt-in image_trust path, never the core proxy path. Grype matches on module version rather than reachability, so once the 2026-09-04 vulnerability DB shipped the finding failed "CI: Verify"'s Docker Build on every branch, starting with #431, and it flags the published v2.1.0 image the same way. govulncheck reports zero reachable vulnerabilities either side of the bump. go build ./... is green on v0.56.0. go mod tidy normalizes the go directive to 1.26.0 in passing.
The comment claimed hardened presets enable RedactHostTopology, but no file under app/configs/ sets redact_host_topology and configuration.mdx documents it as opt-in with a default of false. Rewrite the comment to match what the presets and docs actually do.
GET /images/{name}/json had no entry in the response filter's dispatch
table (ModifyResponse's switch or responseTable), so with every
response.redact_* option enabled the body came back byte-identical,
including Config.Env (build-time environment baked in via Dockerfile
ENV or --build-arg) and GraphDriver.Data (the storage driver's host
filesystem paths for the image's layers). These are the same two leaks
container inspect already closes.
Image inspect now reuses container inspect's existing helpers:
Config.Env is emptied under redact_container_env and GraphDriver.Data
is masked under redact_mount_paths, gated exactly as they are on
container inspect. Image inspect has no Mounts, HostConfig or
NetworkSettings, so redact_network_topology has nothing to do here.
The native libpod route (GET /libpod/images/{name}/json) is wired to
the same handler. Verified against Podman v5.8.1's pinned
containers/common release: libpod.GetImage writes *libimage.ImageData,
whose Config field is *ociv1.ImageConfig (Env carries the identical
json tag Docker's compat handler uses) and whose GraphDriver field is
the same *DriverData{Name, Data} shape redactGraphDriverData already
handles. Both routes' inspect predicates use a last-segment match
rather than isContainerInspectPath's single-Cut split, since an image
reference routinely contains slashes where a container ID never does.
Docs (security.mdx, podman.mdx) now list image inspect among the
redacted routes, and podman.mdx's Known Limitations section drops the
bullet describing this as an open gap. This is a pre-existing gap in
v2.0.0, not a v2.1.0 regression.
build(deps): bump golang.org/x/crypto to v0.56.0 for GO-2026-6354 and GO-2026-6355
The visibility filter, owner isolation's /system/df filter and the response filter all forwarded a 304 Not Modified untouched. A client that fetched a list or an inspect before a policy tightened, or before any policy existed, could revalidate with If-None-Match or If-Modified-Since and have the daemon confirm the copy it already held, so the body it went on using never passed a filter. The validator behind that copy is the daemon's, computed over the unfiltered body, and every axis that would narrow it is reloadable, so the cached copy and the current policy can differ by an arbitrary amount. The strip goes in the reverse proxy's Rewrite rather than in each of the three layers. That is the one point every proxied request passes through exactly once, it is the last point before the wire, and it makes the guarantee independent of which layers are configured. Doing it per layer would need the three to agree on a path predicate, and the response filter has no request-side hook at all: its coverage is a dispatch table, so any predicate a middleware consulted would go stale the moment that table grew an entry. Rewrite edits ReverseProxy's outbound clone, so the client's request and the access and audit record of what it sent are unchanged. A 304 arriving anyway is refused rather than relayed, matching how each layer already treats a body it cannot walk. Each refusal carries its own reason code (visibility_not_modified_unfilterable, owner_not_modified_unfilterable, and the response filter's existing rejectResponse path) so it does not read as a policy lookup that failed, which is a different investigation. 204 keeps its pass-through: it is not a revalidation and has no stale representation behind it. Pre-existing and theoretical. Neither dockerd nor Podman emits ETag or Last-Modified on these routes, so a conditional request against either is already answered with a full 200 and the strip changes nothing observable. The fail-closed claim in the docs should not rest on an upstream detail this proxy does not control.
…d lists HEAD on a route this proxy constrains on the response was forwarded untouched, so the daemon's Content-Length and ETag reached the client describing the unfiltered body. The length counts the containers and images a name or image pattern hides, and the ETag validates them, which makes a HEAD a cheap fingerprint of exactly what the policy was configured to conceal. It covers /containers/json and /images/json plus both /libpod spellings under the pattern axes, and /system/df under either visibility policy or owner isolation, since that route takes no filters parameter and is scoped entirely on the response. The headers are cleared and the request is still forwarded, rather than the route being refused. The refusals this middleware already has are for endpoints no policy axis can scope on any method: /libpod/system/df and the unscopeable libpod reads carry neither labels nor names, GET is refused there too, and HEAD is refused only so a method-scoped gate cannot forward what GET could not. Here GET is fully scoped and only the HEAD's metadata is not, so a refusal would be a per-method status this package has nowhere else, and neither candidate is true: 405 claims the route rejects the method, and 502 blames the upstream for a decision this proxy made. The rule that does fit is already in the package — the client never receives a representation header describing bytes it did not get, which clearUpstreamRepresentationHeaders enforces for the rewritten bodies and for both fail-closed 502 paths. A HEAD is that rule at zero bytes. Go omits Content-Length for a HEAD when the handler declares none and writes nothing, so the response carries no length rather than a fabricated 0; a test over a real server pins that rather than trusting the recorder. Ownership's HEAD /system/df is fixed alongside visibility's because it is the same leak on the same route. The two layers nest, so a deployment running both had it closed by visibility already, but owner isolation without a visibility policy did not. Routes whose selectors are injected into the upstream request are deliberately untouched: the daemon computes their length over the already-scoped list, so there is nothing there to hide.
matchGlobSegments is the allocation-free stand-in for the anchored regex a single-star pattern compiles to, and the two disagreed in both directions on a path ending in "/". The walker absorbed a trailing empty segment after its last pattern segment, so "/containers/*" matched "/containers/abc/" where "^/containers/[^/]*$" does not, and it refused to spend a pattern segment on that empty segment, so "/*/*/*" did not match "/a/b/" where the regex does. Align the walker to the regex: a trailing slash is a real, empty final segment, and a pattern matches only a path carrying exactly as many "/"-separated segments as the pattern spends. The regex is the dialect's definition and the walker is only an optimization, so an optimization that answers differently is the bug by construction. NormalizePath's path.Clean strips a trailing slash, so neither half is reachable on an ordinary Docker route. NormalizePodmanRoutePath deliberately keeps the slash gorilla/mux routes on, so both were reachable on the libpod image-SCP route view, and both were policy bugs there. Absorbing the slash let "allow POST /libpod/images/scp/*" admit "/libpod/images/scp/alpine/", which Podman routes as an SCP of the image "alpine/" rather than of "alpine". Refusing to spend a segment let "deny POST /libpod/images/scp/*/*" miss "/libpod/images/scp/tenant/", so the "allow POST /libpod/images/scp/**" below it fired. Two alternatives were rejected. Aligning the regex to the walker, so a trailing slash is ignored, contradicts why NormalizePodmanRoutePath exists: that slash is what separates the SCP route from the push, tag and untag routes Podman registers earlier. Normalizing the slash away for matching while preserving it on the forwarded path is the same defect wearing a different hat, because "/libpod/images/scp/victim/push/" would then borrow a ".../push" allow and still be routed as an SCP. No shipped preset changes. All 24 presets in app/configs deny every POST /libpod/images/scp/... shape at the decoded view, which is evaluated first and is untouched here, so the route view is never consulted. Nothing legitimate narrows either: an image reference cannot end in "/", and the route view is only computed when the escaped path differs from the cleaned one. Widen the matcher differential corpus to build from both production path views instead of NormalizePath alone, which takes it to 993 paths; measured against the old walker, 512 of its verdicts over the segment-glob patterns disagreed with the regex, and none do now. Add table-driven cases for both shapes and for the libpod SCP route through the production evaluator, carry the walker-versus-regex invariant into FuzzPathMatch on both views, and seed both fuzz corpora with trailing-slash inputs.
…-redaction # Conflicts: # CHANGELOG.md
…-slash # Conflicts: # CHANGELOG.md
The pattern response filter walked the buffered list body element by element and stopped when Decoder.More() went false. That is false both when the array closes and when the input runs out, so the parser never established that the array closed at all, and it never looked at what came after it. A body ending mid-array, or a valid array followed by a second value or by garbage, was rewritten into a well-formed 200 the client read as the complete list. The closing delimiter is now required and any non-whitespace trailing bytes are refused, with the same 502 the non-array case already used. Trailing whitespace still passes. Most truncations did surface already, through the element decode rather than through any check in this parser, so they held by accident of how More() and Decode() interact; trailing bytes were not caught at all. The reasoning is the non-array case's: a body this build cannot account for in full is one whose contents it cannot claim to have checked, so it is not completed on the client's behalf. Elements were always filtered individually, so this was never a confidentiality bypass. What it cost was the documented fail-closed claim, which was narrower than stated. FuzzVisibilityFilter now asserts the refusal itself, with encoding/json's whole-body parse as an oracle against the streaming decoder under test, rather than only bounding output length. That bound could only catch a body that grew, and both gaps here were bodies that were silently completed. Ran clean over 1.6 million executions.
…ditional-and-head # Conflicts: # CHANGELOG.md
The trailing-slash note claimed a two-segment rule covers POST /libpod/images/scp/alpine/ on its own. evaluateRequestPolicy checks the cleaned view first and returns on a non-allow, and the cleaned path has one segment after scp, so a two-segment rule alone denies it. Spell out that both views have to allow, and keep the /** recommendation.
Filter.ModifyResponse rejected every 304 before the method/status gate,
including on writes. It is the single proxy-wide ReverseProxy.ModifyResponse
(wired in cmd/serve.go), so it also intercepted POST /containers/{id}/start
and POST /containers/{id}/stop, both of which the Docker Engine API
documents as legitimately answering 304 ("container already started" /
"container already stopped") when the container is already in the requested
state. Podman's compat and /libpod routes mirror both, plus its own
POST /libpod/containers/{id}/init ("container already initialized").
Checked the full Engine API spec (moby/moby api/docs, through v1.56) and
Podman's route comments (containers/podman pkg/api/server/register_containers.go):
restart, kill, pause and unpause document no 304 response on either API, so
start/stop/init are the whole documented set today.
Rejecting those turned a correct idempotent no-op into a 502 for
orchestrators and retry loops. Move the 304 check to branch on method: GET
and HEAD keep the existing refusal (a 304 there can only mean cache
revalidation, which StripConditionalRequestHeaders means a daemon cannot
legitimately produce), every other method now passes the 304 through
unchanged.
Checked the visibility and ownership 304 backstops added alongside the
original strip: both are unreachable except under a GET/HEAD gate already
(visibility/middleware.go's Middleware returns early for any other method
before reaching patternFilterWriter; ownership/system_data_usage.go's
serveOwnershipAllowed only routes to filterSystemDataUsageResponse on
http.MethodGet), so neither needed scoping.
Adds table-driven cases for start/stop pass-through (plain, /v1.45/-prefixed,
and /libpod) with every redaction option on, plus an end-to-end proxy test
showing a 304 from POST start reaches the client as 304 through the real
ReverseProxy.ModifyResponse wiring.
docs(security): roll the support window to 2.1.x and 2.0.x
docs(config): drop the preset claim from the RedactHostTopology comment
fix(responsefilter): redact image inspect like container inspect
fix(filter): make the glob walker and its regex agree on trailing slashes
…ditional-and-head # Conflicts: # CHANGELOG.md
visibility's forwardHeadWithoutUpstreamRepresentation and ownership's helper of the same name forwarded whatever status the upstream sent on a HEAD, including a recorded 304. Their GET twins already refuse a 304 with a fail-closed 502 (visibility_not_modified_unfilterable / owner_not_modified_unfilterable), because a 304 only means the client's cached copy is current under whatever policy produced it, and neither layer can vouch for that on a HEAD any more than on a GET. Both helpers now check the recorded upstream status before forwarding and apply the same refusal the GET path applies, same reason code and logging, instead of relaying the 304. A real daemon can't reach this branch since conditional headers are stripped before the request reaches it, but the backstop is meant to be unconditional on these routes, matching the fail-closed claim in docs/content/docs/security.mdx. Extends the existing 304 refusal tests in both packages with HEAD cases, plus a real-server test per package confirming the 502 body never actually reaches the wire on a HEAD.
…head fix(isolation): close the 304, HEAD and trailing-bytes gaps on the read side
internal/filter mixed policy evaluation with pure request classification, and internal/ownership, internal/visibility, internal/responsefilter, internal/ratelimit, internal/proxy, internal/config and internal/cmd all imported it partly for the classification half. NormalizePath, CanonicalizePath, StripVersionPrefix, pathNeedsClean, pathSegmentNeedsClean, HasVersionPrefix, NormalizePodmanRoutePath and the hijack-candidate set (IsHijackCandidatePath, IsContainerAttachPath, IsExecStartPath, IsLibpodContainerAttachPath, IsLibpodExecStartPath) move verbatim into internal/apipath, the leaf package #478 created. Each one takes a method or a path string and returns a bool or a string, and reads no config and no policy state. internal/filter keeps the four exported names as one-line wrappers and isExecStartPath/isLibpodExecStartPath as unexported ones, so no call site changed. internal/ratelimit, which imported internal/filter for NormalizePath alone, imports the leaf instead and drops that dependency. Every moved helper's unit tests, mutation-kill tests and benchmarks move with it. The fuzz targets stay in internal/filter, where their persisted corpora and their name-plus-package CI registration live, and reach the same code through the wrappers. No behaviour change.
…classification-leaf-package
streamArrayResponse decoded every array element whole into a map[string]any and re-marshalled it, so a GET /containers/json entry's Names, Image, Command, Ports, Labels, State and Status cost an allocation per string, per nested map and per interface box on the way to being re-encoded byte for byte. Each list route now declares the top-level keys its mutator reads, rewrites or deletes. An element is decoded one level deep into a json.RawMessage per key, only the declared keys are decoded the rest of the way, and every other field is re-emitted from the bytes the daemon sent. The shallow field map, the map the mutator is handed, the scratch the rewritten values are marshaled into and the decoder that reads them are all reused across the elements of one response. Output is unchanged except that untouched values keep their own formatting - a nested object's key order, a \u escape - instead of being canonicalized. Top-level key order, escaping and the values the mutator rewrote are what encoding/json produced before, because the raw field map is what gets encoded. A declared set short of a key its mutator reads is a redaction that silently does not happen, and no route's own tests would catch it, because they go down the same short path. TestListPartialDecodeMatches FullDecode runs every list route through both the partial and the full decode and fails on any difference; it was verified by shortening the container, service and libpod CNI sets, each of which it caught. BenchmarkModifyResponseContainerList over 500 entries, 92,325 allocs/op and 3.90 MB/op before on every profile: 64,485 and 3.24 MB with mount paths and network topology both on, 47,731 and 2.80 MB with topology alone, 31,704 and 1.91 MB with mount paths alone (6.6 ms to 3.8 ms).
…n-leaf-package Merge remote-tracking branch 'origin/dev/v2.1' into refactor/request-classification-leaf-package
…er-pooled-reads-rawmessage
The POST /containers/create inspector allocated a fresh containerCreateRequest per request: ~600 bytes of struct, a backing array for each list field the body carried, and buckets for Labels and Sysctls, all of it garbage before inspect returned. It now takes the target from a sync.Pool and puts it back. The decode is deliberately untouched. It is still the same json.Unmarshal against the same type, so every field the policy reads and every type error a malformed body produces are exactly what they were. Narrowing the struct, which is what PERF-2 originally asked for, would not have been: a field removed from the struct becomes an unknown field, and encoding/json type-checks known fields but skips unknown ones, so dropping the three fields nothing reads (HostConfig.MemoryReservation, DeviceRequests[].DeviceIDs, DeviceRequests[].Options) turns today's malformed-body denial into a pass-through, and all three allocate nothing when absent anyway. BenchmarkInspectContainerCreate on go1.26.6, 20 interleaved runs a side: strict_full_walk 45 -> 36 allocs/op, 7.218 -> 6.178 KiB/op, 5016 -> 4701 ns/op; permissive_early_exit 30 -> 27 allocs/op, 6.897 -> 6.014 KiB/op, 2930 -> 2534 ns/op. The decode on its own goes from 30 allocs and 1568 B to 21 and 496. - perf(filter): pool and reset the containerCreateRequest decode target - test(filter): pin reset exhaustiveness, reuse parity, and per-field decode coverage - docs(changelog): record the measurement and the parity reasoning
…ds-rawmessage Merge remote-tracking branch 'origin/dev/v2.1' into perf/responsefilter-pooled-reads-rawmessage
…eate-targeted-decode
Every request the ownership middleware injects an owner label into ran filter.RejectDuplicateCaseVariantJSONKeys over the raw body and then decoded the same bytes again for the mutation, so each inspected create built two identical map[string]any trees and threw one away. The ambiguity guard now runs against the tree the mutation already decoded, through a new filter.RejectDuplicateCaseVariantJSONValue that wraps the same unexported walk the byte-taking form already delegated to. Same walk, same value, same verdict. Measured over a realistic 3.9 KB POST /containers/create body: 101.45us -> 64.26us (-37%), 79.1 KiB -> 48.4 KiB (-39%), 1415 -> 808 allocations (-43%). - perf(ownership): run the ambiguity check on the decoded tree - feat(filter): add RejectDuplicateCaseVariantJSONValue - test(ownership): guard the single decode and pin the body edge cases - test(filter): lock the two ambiguity entry points to one verdict
…-decode Merge remote-tracking branch 'origin/dev/v2.1' into perf/container-create-targeted-decode
…bel-injection-streaming
…-streaming Merge remote-tracking branch 'origin/dev/v2.1' into perf/ownership-label-injection-streaming
…er-fixtures ci(gitleaks): allowlist the response-filter fixture secrets
Turn the accumulated Unreleased entries into a dated 2.2.0 section, merge the duplicated subheadings the many merges left behind into one each, and point every release-facing surface at 2.2.0: the website version and roadmap, the README banner and roadmap tables, the docs roadmap, the Helm chart version/appVersion with the image tag cleared for prepublication, and SECURITY.md's supported-versions window. The cut is a minor, not the 2.1.1 the roadmap once planned: dev carries four feature commits and a breaking config-validation change, since a match.path without a leading slash now fails validation instead of loading and matching every rooted path.
chore(release): prepare 2.2.0 release metadata
…ay bounded under -race maxCatalogReachabilitySteps was set at 1<<23 against uninstrumented wall time for one exhausted search, about 30ms. That is the wrong measurement. A validateAndCompileRules walk spends a fresh budget per catalog row, so a config that exhausts the cap pays it once per row and not once per config, and CI runs the suite under `go test -race -covermode=atomic`, where the per-block coverage counters cost more on these tight NFA loops than the race detector does. The same walk measures 0.24s plain and 35s under CI's flags, and every dev/v2.1 run since #478 has failed TestValidateAndCompileRulesBoundsLongPatterns/glob-dense_long_pattern at 63-92s against its own 60s deadline. Drop the budget to 1<<20, which takes that case to 0.04s plain, 0.48s under -race and 5.2s under CI's flags. It stays 18x the 56,537 steps the heaviest shipped preset spends on its most expensive catalog row and 2.4x the 443,388 a 1KB literal pattern spends being proved exactly, so no config's verdict moves and the search still fails closed on exhaustion. The transition and state caps are not the binding constraint in either direction and are unchanged. TestCatalogReachabilityStepBudgetKeepsMeasuredHeadroom pins the window the budget has to sit in. Nothing guarded the ceiling before, so a raise back to 1<<23 would only have surfaced as a CI timeout a release later.
…t-ci fix(cmd): tighten the catalog reachability budget so long patterns stay bounded under -race
scttbnsn
requested review from
ALARGECOMPANY and
biggest-littlest
as code owners
September 6, 2026 13:23
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 230 files, which is 130 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (230)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
test(cmd): clear SOCKGUARD_* before asserting clean pre-validation stderr
biggest-littlest
approved these changes
Sep 6, 2026
biggest-littlest
left a comment
biggest-littlest
left a comment
Member
There was a problem hiding this comment.
Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.
ALARGECOMPANY
approved these changes
Sep 6, 2026
ALARGECOMPANY
left a comment
ALARGECOMPANY
left a comment
Member
There was a problem hiding this comment.
Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.
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
3 participants
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.
Promotes the v2.2.0 release line to main. Everything on dev/v2.1 since v2.1.0 landed through PRs #443 to #487, hook-gated and CI-verified: the volume-mount and read-side gaps (#458, #465, #468, #461, #462), the request-target and rule-pattern fixes (#459, #464), the operator-facing additions (
sockguard verify#474,server.shutdown_grace#472, unknownSOCKGUARD_*warning #469), the CI and packaging work (#466, #470, #471), the refactor and perf batch (#475 to #484), the CI fixes that followed (#485, #487) and the release metadata (#486). The last dev Verify run on 273ab98 is green across all 13 jobs.Minor, not patch, because of the four features. One change is breaking for configs that spelled a
match.pathwithout a leading/: that shape now fails validation instead of loading and matching every rooted path. No shipped preset, Tecnativa-compat rule or documented pattern uses it; the release intro in CHANGELOG says so.Straight to GA, no rc: the tri-tool conformance workflow gets dispatched against the published 2.2.0 image after the cut.
Merge as a merge commit, not a squash, so the reconcile ancestry holds.