fix(responsefilter): redact native libpod responses - #387
Draft
scttbnsn wants to merge 14 commits into
Draft
Conversation
scttbnsn
commented
Aug 29, 2026
scttbnsn
commented
Contributor
- fix(responsefilter): redact Podman's native libpod read responses
- fix(responsefilter): decode list responses through the shared decoder
- style(responsefilter): apply canonical Go formatting
NormalizePath strips the Docker API version prefix but not /libpod, so every
Docker-compat predicate in this package failed to match the Podman spelling
of the same read and the whole package was a no-op on /libpod/....
redact_container_env, redact_mount_paths, redact_network_topology and
redact_sensitive_data all default to true, and all four were silently doing
nothing there:
GET /libpod/containers/{id}/json Config.Env, Mounts[].Source,
HostConfig.Binds, NetworkMode and the
whole NetworkSettings address block
GET /libpod/volumes/{json,{n}/json} Mountpoint
GET /libpod/networks/{json,{id}/json,{id}}
subnets, routes, network_dns_servers,
network_interface, and inspect's
cross-owner containers map
GET /libpod/secrets/{json,{n}/json} SecretData
configs/podman-readonly.yaml allows all of those as shipped, so a monitoring
deployment that changed nothing was leaking.
Route the libpod family whole, before any Docker-compat predicate gets a
look, so a handler can never be reached by a near-miss match on a body shape
it was never checked against. Reuse a compat handler only where the field
names were checked against Podman v5.8.1's own types, which is container
inspect and volume inspect and nowhere else.
The other shapes are genuinely different. Podman's network object shares no
key with Docker's, so a redactor pointed at the libpod path without the
libpod field names would have decoded the body, matched nothing and shipped
it. Both libpod list bodies are bare arrays where the Docker-compat volume
list is a {"Volumes":[...],"Warnings":[...]} object. And the secret plaintext
is not Spec.Data at all: entities.SecretSpec has no Data field, and
SecretInfoReport.SecretData sits one level up, filled by abi.SecretInspect
from LookupSecretData whenever the query carries showsecret=true. That one
leaks on Podman's Docker-compat surface too, because compat.InspectSecret
reads showsecret before it branches on IsLibpodRequest and the compat reply
type embeds SecretInfoReport, so both fields are now redacted on both
surfaces. Docker's own daemon never emits SecretData, so the addition is
inert against dockerd.
register_networks.go points both GET /libpod/networks/{name}/json and a bare
GET /libpod/networks/{name} at libpod.InspectNetwork, on consecutive lines,
with a swagger block on only the first. Matching the documented spelling
alone would have left the identical body reachable one path segment away, so
the bare form is matched too: GET-only, and excluding the three
collection-level segments (json, create, prune) so POST create's reply keeps
the subnet the caller just chose. Inspect also accepts the single-element
array envelope the handler wrote through v3.0.0 before switching to
reports[0] at v3.1.0, and re-emits whichever one it was given.
Five libpod reads stay unrewritten on purpose, each checked against Podman's
types rather than assumed: containers/json has no host source in it
(ListContainer.Mounts is a []string of destinations), info shares no field
with the Docker Info redactors, pods/{id}/json needs a handler of its own,
and system/df is refused by ownership and visibility instead.
containers/showmounted is the fifth and is new information: it answers with a
map of every container on the host to its host mountpoint, so redacting the
values would leave the enumeration and the answer there is a refusal rather
than a rewrite, which nothing implements yet. The Podman guide names all
five, plus image inspect, where libpod and Docker-compat are equally
unredacted.
The precision fix and the trailing-document fix both landed on "the five
decode sites". There were six. streamArrayResponse decodes array-shaped
responses one element at a time instead of reading the whole body, so it
constructed its own json.NewDecoder and was never folded in, and it kept both
defects after the other five lost them.
Numbers first. Without UseNumber, encoding/json coerces every JSON number to
float64, so an integer above 2^53 is re-encoded with different digits than the
daemon sent. Measured on GET /containers/json with redact_mount_paths, which
defaults to true:
in [{"Id":"c-a","Mounts":[...],"SizeRootFs":9007199254740993}]
out [{"Id":"c-a","Mounts":[...],"SizeRootFs":9007199254740992}]
Same for /networks, every responseTable list entry (/services, /tasks,
/secrets, /configs, /plugins, /nodes) and the libpod arrays. This is a
client-facing path: streamArrayResponse replaces resp.Body with what it
re-encoded, so the corrupted digits are what the client reads.
Then trailing content. json.Decoder stops at the end of the first value, so
a body carrying a second document after the array was truncated to the first
one and forwarded with no error:
in [{"Id":"c-a"}][{"Id":"SMUGGLED"}] out [{"Id":"c-a"}] err=nil
in [{"Id":"c-a"}] x out [{"Id":"c-a"}] err=nil
decodeJSONObject has rejected that since the whole-body sites were
consolidated. A filter that decides what a client may see cannot silently
drop the rest of a body, so the array path now consumes the closing bracket
and runs the same trailingJSONError over what follows. Truncated arrays
already failed closed, because dec.More reports true and the next Decode
returns the EOF; that is unchanged and pinned. An unbalanced [{"a":1}} was
being repaired to a valid array and is now refused.
Fix the asymmetry rather than the two symptoms. Two decoder configurations
that must agree eventually will not, and nothing marked which was which. The
configuration now lives in one newJSONDecoder helper that decodeJSONObject,
decodeJSONObjectArray and streamArrayResponse all call, and json.NewDecoder
appears exactly once in the package.
|
Deployment failed for project sockguard-website with the following error: Learn More: https://vercel.com/codeswhat?upgradeToPro=build-rate-limit |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
scttbnsn
changed the title
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.