Skip to content

Commit 3c72f8c

Browse files
committed
test(sealed-bridge): skip when no builder-valid system tool exists, do not stub the guard
The prior fix monkeypatched builder._require_safe_ancestry for the whole test class. For system_only it checked only S_ISDIR and the write bits, dropping the uid check that raises "ancestry is not owned by current uid or root" - so /usr, the exact path failing on the runner, passed inside the tests. The product file was untouched, but nothing exercised the ownership dimension of that rule any more, so a regression in it would ship silently. Green obtained that way is a false signal. The selector now enforces the builder's own validation, which carries the ancestry rule, plus the manifest's separate root-ownership requirement, and returns None when nothing qualifies. setUp raises SkipTest with the reason. Two alternatives were measured and rejected. A test-created fallback cannot work: load_manifest requires st_uid == 0 unconditionally for system tools and a test process cannot own a file as root - that yields "must be root-owned" instead. An ancestry-aware selector with no skip cannot work either, because every candidate resolves under the failing /usr and next() finds nothing. Verified: a normal host runs 55/55 with zero skips, so coverage is unchanged wherever the environment is sound; under a faithful runner shape the result is 0 failures, 0 errors, 55 skipped with the reason stated. A skip is visible and explains itself; a stub is invisible. The product ancestry check is untouched. The underlying cause is a GitHub runner-image change that affects main equally.
1 parent c799fd0 commit 3c72f8c

1 file changed

Lines changed: 45 additions & 38 deletions

File tree

tests/test_build_sealed_bridge_runtimes.py

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -30,53 +30,64 @@ def sha(path: Path) -> str:
3030
return hashlib.sha256(path.read_bytes()).hexdigest()
3131

3232

33-
def host_system_tool_paths() -> dict[str, Path]:
33+
def host_system_tool_paths() -> dict[str, Path] | None:
34+
"""Map each pinned system tool to a host path the BUILDER will also accept.
35+
36+
The fixture must never accept a tool the builder then rejects: every case
37+
would die on that rejection instead of its own assertion. So usability is the
38+
builder's own validation - which carries the ancestry rule - plus the
39+
manifest's separate root-ownership requirement (load_manifest refuses any
40+
system tool whose st_uid is not 0).
41+
42+
Returns None when nothing qualifies. That is a real host condition rather
43+
than a bug: a runner image whose /usr is owned by neither root nor the
44+
invoking uid leaves no usable root-owned system tool, and a test-created
45+
fallback cannot stand in because a test process cannot own a file as root.
46+
Callers skip, so the gap is visible and states its reason. Do NOT substitute
47+
a relaxed ancestry rule here - that would silently stop exercising the
48+
ownership property these tests exist to cover.
49+
"""
50+
3451
def usable(path: Path) -> bool:
3552
try:
3653
metadata = path.lstat()
3754
except OSError:
3855
return False
39-
return (
56+
if not (
4057
stat.S_ISREG(metadata.st_mode)
4158
and metadata.st_uid == 0
4259
and not stat.S_IMODE(metadata.st_mode) & 0o022
4360
and Path(os.path.realpath(path)) == path
4461
and os.access(path, os.X_OK)
45-
)
62+
):
63+
return False
64+
try:
65+
builder._require_regular(
66+
path, "fixture system tool", executable=True, allow_root_owner=True
67+
)
68+
except (OSError, builder.BuildError):
69+
return False
70+
return True
4671

4772
fallback = next(
48-
path
49-
for path in map(
50-
Path,
51-
("/usr/bin/true", "/bin/true", "/usr/bin/env", "/bin/echo"),
52-
)
53-
if usable(path)
73+
(
74+
path
75+
for path in map(
76+
Path,
77+
("/usr/bin/true", "/bin/true", "/usr/bin/env", "/bin/echo"),
78+
)
79+
if usable(path)
80+
),
81+
None,
5482
)
83+
if fallback is None:
84+
return None
5585
return {
5686
name: path if usable(path) else fallback
5787
for name, path in builder.SYSTEM_TOOL_PATHS.items()
5888
}
5989

6090

61-
def require_safe_fixture_ancestry(
62-
path: Path,
63-
label: str,
64-
*,
65-
system_only: bool = False,
66-
) -> None:
67-
if not system_only:
68-
builder._ORIGINAL_REQUIRE_SAFE_ANCESTRY(path, label)
69-
return
70-
current = path
71-
while True:
72-
info = os.lstat(current)
73-
if not stat.S_ISDIR(info.st_mode) or stat.S_IMODE(info.st_mode) & 0o022:
74-
raise builder.BuildError(f"{label} has unsafe system ancestry: {current}")
75-
if current == current.parent:
76-
return
77-
current = current.parent
78-
79-
8091
class ManifestFixture:
8192
def __init__(self, temporary: Path) -> None:
8293
self.root = temporary
@@ -199,24 +210,20 @@ class SealedRuntimeBuilderTests(unittest.TestCase):
199210
def setUp(self) -> None:
200211
self.temporary = tempfile.TemporaryDirectory(prefix="sealed-builder-test-")
201212
self.root = Path(os.path.realpath(self.temporary.name))
202-
if not hasattr(builder, "_ORIGINAL_REQUIRE_SAFE_ANCESTRY"):
203-
builder._ORIGINAL_REQUIRE_SAFE_ANCESTRY = builder._require_safe_ancestry
204-
self.system_ancestry_patch = mock.patch.object(
205-
builder,
206-
"_require_safe_ancestry",
207-
side_effect=require_safe_fixture_ancestry,
208-
)
209-
self.system_ancestry_patch.start()
213+
system_tools = host_system_tool_paths()
214+
if system_tools is None:
215+
raise unittest.SkipTest(
216+
"no root-owned system tool with builder-safe ancestry on this host"
217+
)
210218
self.system_tools_patch = mock.patch.object(
211-
builder, "SYSTEM_TOOL_PATHS", host_system_tool_paths()
219+
builder, "SYSTEM_TOOL_PATHS", system_tools
212220
)
213221
self.system_tools_patch.start()
214222
self.fixture = ManifestFixture(self.root)
215223

216224
def tearDown(self) -> None:
217225
builder._unseal(self.root)
218226
self.system_tools_patch.stop()
219-
self.system_ancestry_patch.stop()
220227
self.temporary.cleanup()
221228

222229
def test_manifest_accepts_exact_four_role_schema(self) -> None:

0 commit comments

Comments
 (0)