fix: close the 3 remaining review findings (config_hash mask, podman localhost/, rootless caveats)#71
Merged
Merged
Conversation
…localhost/, rootless caveats) Post-review cleanup of the pre-existing findings review-launcher surfaced on #67 (the HIGH chown one shipped as #69); none are regressions. - build(): hoist `_chash="$(config_hash)"` out of the `local args=(...)` array. local returns 0, so a failing hash (shasum rc 127) baked an empty confighash label under set -e, after which every maybe_build silently rebuilt. Assigned alone the failure propagates and the build dies. - ls/prune: strip podman's `localhost/` image-name prefix at the two `image ls` seeds so the name matches the canonical sluice-<slug> everywhere downstream (ps status filter, box_blocked_count, the ${#sluice-} receipt slug, remove_box_volumes label) - it was breaking all of them on podman. No-op on docker (never emits the prefix). - doctor: a shared _rootless_podman helper surfaces the rootless-podman host limits - pids/memory caps need cgroups-v2 + systemd delegation, ports <1024 can't bind (flagged per configured port), netfilter modules must be loaded; additive rootless_podman JSON field; docs/hardening.md documents all three. Tests: verify-build-hash-unit.bats (new), + podman-prefix and rootless-doctor cases in verify-ls-egress-unit / verify-doctor-checks. Unit 318 green, acceptance 16 green; podman-specific paths ride the best-effort podman CI leg.
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.
Post-review cleanup of the pre-existing findings
review-launchersurfaced on #67. The HIGH one (rootless chown) shipped as #69; these are the remaining five, none regressions, bundled per the one-PR decision.Fix 1 — config_hash failure masked in build() (LOW, bash-3.2)
local args=(--label "sluice.confighash=$(config_hash)" ...)returns local's own exit 0, so a failing hash (shasumrc 127) baked an empty confighash label underset -e— after which everymaybe_buildsaw a mismatch and rebuilt silently. Hoisted tolocal _chash; _chash="$(config_hash)"(mirroringmaybe_build), the failure now propagates and the build dies.Fix 2 — podman
localhost/image-name prefix breaks ls/prune (MEDIUM)sluice ls/pruneseed fromimage ls --format '{{.Repository}}'; podman printslocalhost/sluice-foo, which then mismatched the canonicalsluice-fooin thepsstatus filter,box_blocked_count, the${#sluice-}receipt slug, andremove_box_volumes's label — so on podman boxes never showed running, last_receipt/state_dir were wrong, and overlay volumes leaked on prune. Both seeds nowsed 's,^localhost/,,'(a no-op on docker).Fix 3 — surface the 3 rootless-podman limitations (3× LOW)
A shared
_rootless_podmanhelper (also DRYs #69's detection) drives newsluice doctorcaveats: pids/memory caps enforce only with cgroups-v2 + systemd delegation; host ports <1024 can't bind (flagged per configured port); netfilter modules must be loaded. Additive"rootless_podman"JSON field for parity;docs/hardening.mddocuments all three (the module one is doc-only — the host-side doctor can't probe the box's future netns).Tests / verification
verify-build-hash-unit.bats(new): a failingconfig_hashaborts the build underset -e; structural hoist check.verify-ls-egress-unit.bats: alocalhost/-prefixed image resolves to the canonical name + state_dir; structural (both seeds strip).verify-doctor-checks.bats: rootless caveats appear (fake podman on PATH), docker emits none, JSONrootless_podmantrue/false.