✨ feat(filter): libpod container-create body inspector with golden fixtures (#148) - #191
Conversation
Real POST /libpod/containers/create request bodies captured from a live podman (client 6.0.2 darwin/arm64) talking to a podman machine VM (Podman Engine 5.8.1 linux/arm64), covering basic create, privileged, host netns/pidns/ipcns/userns, container:<ref> namespace sharing, bind/tmpfs mounts, named volumes, devices, cap_add/cap_drop, seccomp/apparmor/selinux opts, resource limits (including cpu.shares-only), systemd mode, custom idmappings, labels, sysctls, read-only rootfs, and non-root user. Resolves the field-name ambiguity the design doc left open (ipcns, not ipc) and surfaces several real-world quirks no draft anticipated: the mounts/volumes field-casing split, singular "sysctl" naming, unsplit device path strings, and systemd defaulting to "true" even when --systemd is never passed on the CLI. README.md documents exact client/server versions and per-fixture capture provenance.
libpod_container_create_types.go / libpod_container_create.go implement
POST /libpod/containers/create body inspection, structured like
container_create_types.go/container_create.go: subset decode of the
SpecGenerator body, a policy struct, and an inspect() method gated by
isLibpodContainerCreatePath (path-exclusive — never reads a Docker-shaped
body, and the Docker inspector never reads a libpod-shaped one).
Gates mirror ContainerCreateOptions where the semantics map: privileged,
host netns/pidns/ipcns/userns, namespace-sharing restriction reading
{nsmode,value}, allowed_bind_mounts via the shared normalizeBindMount,
devices, allowed_capabilities, seccomp/apparmor/selinux allowlists
(including selinux disable), require_non_root_user, memory/cpu/cpu-hard/
pids limits from resource_limits, sysctls, read_only_filesystem, and
image trust reusing the existing ImageTrust wiring on the image field.
Two libpod-only gates have no Docker analog: allow_systemd_mode (denies
non-"false" systemd by default, since SpecGenerator's own default is
"true") and allow_custom_id_mappings. Field names and gate semantics are
pinned from the golden fixtures captured in the prior commit, not from
either draft of the design doc.
Adds LibpodContainerCreateRequestBodyConfig under request_body under a new libpod_container_create mapstructure key, following ContainerCreateRequestBodyConfig's field/tag pattern. ToFilterOptions plumbs it into filter.LibpodContainerCreateOptions, and validateLibpodContainerCreateConfig mirrors validateContainerCreateConfig (image_trust validation, allowlist sanity checks). All defaults fail closed; ImageTrust.RequireRekorInclusion defaults to true, matching container_create's default.
middleware.go registers the libpod inspector in compileRuntimePolicy's policy list, keyed by isLibpodContainerCreatePath (path-exclusive, right after the Docker container-create entry). cmd/rules.go adds POST /libpod/containers/create to bodySensitiveWriteEndpoints and a bodyInspectionConfiguredForEndpoint arm, so the startup blind-write validator recognizes the new inspector the same way it does every other body-sensitive endpoint. Kept minimal and additive — a sibling PR is touching the same three files (pod-create/exec/volume inspectors) and will need a rebase against this.
Per-gate table tests driven off the golden fixtures for every one of the ~20 gates in libpod_container_create.go, plus: - TestInspectorRoutingIsPathExclusive: the design doc's central test — a single body carrying both a Docker-shaped dangerous value (HostConfig.Privileged) and a libpod-shaped safe value (top-level "privileged") with opposing truth values (and the mirror body), proving neither inspector ever reads the other's shape, and that each is a structural no-op on the other family's path. - Fail-closed decode tests: malformed JSON, oversized body (1 MiB cap via requestRejectionError), empty body allowed, and nil-request/wrong-method/ wrong-path/nil-body no-ops. - Image-trust tests reusing the existing mockImageVerifier/ mockSignatureFetcher fixtures from container_create_test.go, including the verify→pin digest rewrite and duplicate-case-variant-key rejection. - FuzzLibpodContainerCreate, seeded from all 20 fixtures plus adversarial inputs; 20s local smoke run (~1.3M execs) found no crashes. Every captured fixture defaults "systemd":"true" (SpecGenerator's own default even when --systemd is never passed), which the new allow_systemd_mode gate denies by default — so every "expect allow" assertion in this file explicitly opts a policy into AllowSystemdMode where the fixture would otherwise trip that gate first and mask the gate actually under test. Assertions expecting deny from a gate that runs before the systemd check in inspect()'s order don't need it.
Adds request_body.libpod_container_create to the YAML sample, a prose paragraph describing the gates (mirroring the container_create section's structure), a Request Body Policy Reference table row, and a dedicated environment-variable table following the container_create precedent. CHANGELOG gains an Unreleased ### Added entry summarizing the new inspector, its gate set, and the fixture-derived field-name resolution.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (30)
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 |
PR 2 of the #148 train, building on the merged routing groundwork (#189).
What's here
POST /libpod/containers/createbodies captured from a live Podman socket (client 6.0.2 darwin/arm64, server engine 5.8.1 linux/arm64), committed undertestdata/libpod/with per-file provenance. The fixtures settled shape questions the design drafts disagreed on (ipcnsis the real tag) and surfaced quirks no draft anticipated: the mounts/volumes casing split, singularsysctl, unsplit device paths, andsystemddefaulting to"true"even when never passed — that last one is why the systemd gate reads the effective value, not just presence.libpod_container_create.go+ types): path-exclusive (gated byisLibpodContainerCreatePath), subset decode via the shared JSON helper, ~20 gates mirroringcontainer_create's operator vocabulary (privileged, host namespaces via the{nsmode,value}shape, namespace-sharing restriction, bind-mount allowlist through the sharednormalizeBindMount, devices, capabilities, seccomp/apparmor/selinux including selinux disable, non-root user, memory/CPU/CPU-hard/PIDs fromresource_limits, sysctls, read-only rootfs, image-trust onimage) plus two libpod-only gates:allow_systemd_modeandallow_custom_id_mappings. Same 1 MiB body cap.TestInspectorRoutingIsPathExclusive— the design's central proof: one body carrying both Docker-shaped and libpod-shaped privilege keys with opposing values, run against both paths, each inspector reads only its own shape. A mis-shaped decode reads zero values ("safe"), so this is the fail-open class the whole structure exists to prevent.request_body.libpod_container_create(fail-closed defaults, validation per the existing pattern), blind-write/inspection-configured table entries,FuzzLibpodContainerCreateseeded from the fixtures (~1.3M execs clean), configuration.mdx section + env-var table.Verification
CI-mirrored measurement: production coverage 96.2% vs the 96% floor;
go test -race ./...green;golangci-lint0 issues; full pre-push hook suite green.Part of #148 (does not close it). Will need a rebase against #190 (shared wiring files) — whichever merges second.