docs/cc/e2e: check the shim by content, and force its rebuild

docs/cc/backlog.md

@@ -216,3 +216,4 @@ artifact API.
 | RM-113 | ✅ **`sandbox_id` was the one host-supplied path component the new content-channel handlers did not validate.** In `copy_single_file`, `req.sandbox_id` is joined into the destination (`single-files/<sandbox_id>/<guest-chosen-name>`) without the `is_safe_relative_path` check the three sibling handlers apply to `file_name`, `revision`, and `agent_volume_id`. Not an escape from the shared directory — `do_copy_file` resolves through `pathrs::Root`, which confines writes — but `pathrs` **clamps** a leading `..` at the root rather than rejecting it, so a malicious host could redirect the write out of its own per-sandbox subdirectory and over another sandbox's single-file content inside the same share. Fixed by validating `sandbox_id` identically, with a comment recording why containment alone is not sufficient. | F-210 |
 | RM-114 | ✅ **The new content-channel handlers passed `file_mode` through unvalidated, so the host could create a symlink at the exact path that gets bind-mounted into the container.** `do_copy_file` inspects the S_IFMT bits of `file_mode` and, when `S_IFLNK` is set, creates a symbolic link whose target is the request's `data` rather than writing a file. Neither `copy_single_file` nor `put_volume_file` constrained those bits: both build a `CopyFileRequest` with `file_mode: req.file_mode` and hand it straight to `do_copy_file`. A host could therefore make `single-files/<sandbox_id>/resolv.conf` a symlink to any absolute path and have the container's `/etc/resolv.conf` resolve there — likewise for `/etc/hosts`, `/etc/hostname`, the termination log, and any file inside a projected ConfigMap/Secret/downward-API volume. This is a regression against the very rule the redesign deleted: `allow_copy_file` had a dedicated symlink clause requiring the link to sit below the top level of the shared directory, to be normalised, and to be relative — and these destinations are always *on* that top level, i.e. exactly the case the old rule refused outright. Containment does not help, because the link is created inside the shared directory and it is the container's later resolution of it, not the write, that escapes. Also unconstrained were the setuid/setgid/sticky bits, which `do_copy_file` preserves (`FILE_PERMISSION_MASK` is `0o7777`, not `0o777`). Fixed by a policy-independent guard from @danmihai1: `copy_single_file` and `put_volume_file` both refuse any request whose `file_mode` S_IFMT bits are not `S_IFREG`, before policy is consulted, so the symlink path is unreachable regardless of the installed policy. The same change closes the host-side half -- `copy_single_file_to_guest` read the mount source with `std::fs::metadata` on the *canonicalized* path, silently following a symlinked source; it now uses `symlink_metadata` on the raw mount path. A policy-mediated version was written first (decoding the mode into a `FileType` and surfacing `symlink_target`) and then reverted: with the guard in place those fields could only ever hold `Regular` and `None`, so the rules restating them were dead weight. What the guard does *not* cover is the permission bits, which `do_copy_file` preserves as `file_mode & 0o7777`, so `rules.rego` still rejects setuid, setgid and the sticky bit -- and the payload is still stripped before evaluation, since it was otherwise serialised into the rules engine on every chunk. @danmihai1 independently added a second, policy-independent guard on the same two handlers, which is what upstream now carries: both reject outright any `file_mode` whose S_IFMT bits are not `S_IFREG`, so the symlink path is unreachable even under a permissive or absent policy. The same change closes the host-side half — `copy_single_file_to_guest` read the mount source with `std::fs::metadata` on the *canonicalized* path, which silently followed a symlinked source; it now uses `symlink_metadata` on the raw mount path and refuses anything that is not a regular file. | F-211 |
 | RM-115 | ✅ **A volumeMount's `readOnly` request was unenforceable when its destination collided with a mount the settings also apply.** Raised by @danmihai1 as "CoCo Policy allows 2 or more container mounts for the same destination"; confirmed, and it is a `CreateContainer` policy issue rather than a CopyFile one. Two facts compose. The mount check in `allow_by_bundle_or_sandbox_id` (`rules.rego:889-894`) requires `count(p_matches) == count(i_base_mounts)` over a set of policy indices, which rejects two presented mounts collapsing onto a single policy mount but is an injection rather than a bijection: it does not require every policy mount to be presented, and it constrains no ordering. Probes added to the upstream `createcontainer/volumes/emptydir` fixture confirmed both — omitting an allowed mount and reversing the mount order each evaluate as allowed. Separately, `get_config_map_mount_and_storage` (`mount_and_storage.rs:376-382`) pushed its mount unconditionally, and was the only volume handler that did not first look for an existing entry with the same destination; `get_settings_mounts` (:59), `get_host_path_mount` (:331), `add_shared_bind_mount` (:436), `get_downward_api_mount` (:483) and `get_image_mount_and_storage` (:515) all do. It handles both configMap and secret volumes. A pod mounting a configMap at `/etc/hosts` therefore produced both an `rw` entry and an `ro` entry for that destination, identical apart from the access option, and the host could present the `rw` one and drop the `ro` one — or order `rw` last so that it shadows — leaving the container with a writable `/etc/hosts` although the pod spec asked for read-only. The same applied to `/etc/resolv.conf`, `/etc/hostname` and `/dev/termination-log`. A corpus-wide sweep — every yaml fixture in the genpolicy tests, generated under all three `emptydir_type` settings, 243 containers — then found a second site of the same kind: `get_guest_empty_dir_mount_and_storage` (`:274`), which handles the `Memory` medium and both block-backed emptydir types, `block-encrypted` being the **default**. An emptyDir marked `readOnly` over `/etc/hosts` produced the same `rw`/`ro` pair. Both sites now go through a shared `replace_or_push_mount` helper, with six unit tests. `hostPath`, `persistentVolumeClaim`, `projected` and `downwardAPI` were already correct. After the fix the sweep reports duplicates in exactly two containers, both in `shared-fs` mode and both the deliberate alternation described below. The check itself is untouched upstream code (`fb815b77c1`, also in `microsoft/main`), so nothing in this branch introduced it. **Residual:** in `shared-fs` emptydir mode `get_host_empty_dir_mount` still emits two entries per destination, but deliberately — they are alternative *sources* for one path and carry identical access options (the sweep confirms it: a `readOnly` volumeMount yields `ro` in **both** entries), so no read-only request is weakened. That residual is closed by the second half of the fix: `allow_by_bundle_or_sandbox_id` now also requires `mount_destinations_are_distinct(i_oci)`, so whichever alternative source the host chooses, the container sees exactly one mount per path. This is the analogue of what C-ACI does via `data.metadata.p9mounts` in `plan9_mount`, which tracks mounted targets as policy state, and it removes the injection-not-bijection property as an enabling condition rather than only removing today's duplicates. All 79 requests carrying `OCI.Mounts` across the testcase corpus were checked first: exactly two have duplicate destinations and both are already-denied negative cases, so the rule denies nothing that was legitimately allowed. A new fixture, `createcontainer/volumes/emptydir_shared_fs`, isolates it — `shared-fs` is the only mode where a Policy legitimately holds two entries per destination — with a positive case using one source and a negative case presenting both. Mutation-testing the rule out flips the negative case to allowed, confirming it does not merely re-test the count check. |
+| RM-117 | ✅ **Stage 04's shim currency check failed for a correct shim after a rebase-merge, and could not have forced the rebuild it demanded.** Two faults compose. The check required the commit stamped into the installed shim to be an ancestor of HEAD; when a PR lands by rebase-merge the merged commits are rewritten, so a shim built from the pre-merge branch reports a commit that is in no history at all, and the stage died with "reports commit X, which is not in this branch's history" for a binary whose inputs were identical to HEAD's. The check now compares the *content* of the shim's inputs (`src/runtime-rs`, `src/libs`, `src/dragonball`, `Cargo.toml`, `Cargo.lock`) between that commit and HEAD, which is the question it was always trying to ask and is strictly stronger: it still fails for a genuinely stale shim, reachable commit or not. It also strips the `-dirty` suffix before asking git about the object, which the old form would have choked on. Second, the stamp could lag indefinitely. `src/runtime-rs/Makefile` builds from `$(TARGET_PATH): $(SOURCES)` and does not list `$(GENERATED_FILES)` among those prerequisites, so deleting `crates/shim/src/config.rs` — where `@COMMIT@` is substituted — restamps the source but never makes cargo relink. With no tracked source change the binary kept a stamp from many branches earlier, which is exactly the state that tripped the ancestry test. Stage 04 now removes the build output as well, so make must remake it whatever the timestamps say and the stamp always describes the tree it was built from. |

docs/cc/e2e/04-build-guest-stack.sh

@@ -397,9 +397,17 @@ if [[ -e "${SHIM_DST}" ]]; then
   # it costs one sed and one relink, and is what makes the assertion mean
   # anything at all.
   rm -f src/runtime-rs/crates/shim/src/config.rs
+  # Deleting config.rs restamps the source but does NOT force a relink: the
+  # Makefile builds from `$(TARGET_PATH): $(SOURCES)`, and $(GENERATED_FILES) is
+  # not among those prerequisites. So when no tracked source changed, cargo has
+  # nothing to do and the installed binary keeps whatever commit it was stamped
+  # with, potentially many branches ago. Removing the output too closes that
+  # gap: make must then remake $(TARGET_PATH) whatever the timestamps say, and
+  # the stamp always describes the tree it was actually built from.
+  SHIM_SRC="${E2E_REPO_DIR}/target/x86_64-unknown-linux-musl/release/containerd-shim-kata-v2"
+  rm -f "${SHIM_SRC}"
   ( cd src/runtime-rs && make ) || die "could not build the runtime-rs shim"
 
-  SHIM_SRC="${E2E_REPO_DIR}/target/x86_64-unknown-linux-musl/release/containerd-shim-kata-v2"
   [[ -x "${SHIM_SRC}" ]] || die "runtime-rs build produced no shim at ${SHIM_SRC}"
 
   sudo cp "${SHIM_DST}" "${SHIM_DST}.bak.$(date +%s)"
@@ -408,16 +416,35 @@ if [[ -e "${SHIM_DST}" ]]; then
   # HEAD: cargo rightly does not relink for a commit that touched only docs, so
   # the binary would be stale-but-correct and the stage would fail for no reason.
   # What matters is that nothing the shim is built *from* has changed since.
+  #
+  # Compare content, not ancestry. Requiring the reported commit to be an
+  # ancestor of HEAD looks equivalent but fails the moment a PR lands by
+  # rebase-merge: the merged commits get new SHAs, so a shim built from the
+  # pre-merge branch reports a commit that is no longer in any history even
+  # though its inputs are identical to HEAD's. That produced a stage-04 failure
+  # ("reports commit X, which is not in this branch's history") for a shim that
+  # was in fact correct. A tree comparison answers the question the check is
+  # actually asking, and stays strictly stronger: it fails for a genuinely stale
+  # shim whether or not the commit is reachable.
+  #
+  # Note the stamp can lag legitimately. src/runtime-rs/Makefile builds the
+  # binary from `$(TARGET_PATH): $(SOURCES)`, which does not list
+  # $(GENERATED_FILES) -- so regenerating config.rs (where @COMMIT@ is
+  # substituted) does not by itself make cargo relink. Deleting config.rs above
+  # therefore restamps the source but cannot force a rebuild when no source
+  # changed, which is exactly when the stamp is allowed to lag.
   SHIM_INPUTS=(src/runtime-rs src/libs src/dragonball Cargo.toml Cargo.lock)
   got=$("${SHIM_DST}" --version 2>&1 | sed -n 's/.*commit: *\([0-9a-f]\{7,\}\).*/\1/p')
   [[ -n "${got}" ]] || die "installed shim does not report a commit — is it the runtime-rs shim?"
-  git merge-base --is-ancestor "${got}" HEAD 2>/dev/null \
-    || die "installed shim reports commit ${got}, which is not in this branch's history"
-  stale=$(git log --oneline "${got}..HEAD" -- "${SHIM_INPUTS[@]}")
-  [[ -z "${stale}" ]] || {
-    printf '%s\n' "${stale}" | sed 's/^/    /'
-    die "shim was built at ${got:0:12} but the above commits changed ${SHIM_INPUTS[*]} since — the build did not pick them up"
-  }
+  # The stamp carries a "-dirty" suffix when the tree had uncommitted changes;
+  # strip it before asking git about the object.
+  got="${got%%-dirty}"
+  git cat-file -e "${got}^{commit}" 2>/dev/null \
+    || die "installed shim reports commit ${got}, which this repository does not have — fetch it, or rebuild the shim"
+  if ! git diff --quiet "${got}" HEAD -- "${SHIM_INPUTS[@]}"; then
+    git diff --stat "${got}" HEAD -- "${SHIM_INPUTS[@]}" | sed 's/^/    /'
+    die "shim was built at ${got:0:12} but the above ${SHIM_INPUTS[*]} inputs differ from HEAD — the build did not pick them up"
+  fi
   ok "runtime-rs shim installed and current (built at ${got:0:12})"
   # containerd caches nothing about the shim binary, but any shim already
   # running for a live sandbox is the old one; stage 07 creates fresh pods.