Skip to content

[quality] test-infrastructure: test_save_perms.py assumed zero ambient xattrs, breaking on SELinux-labeled hosts #289

Description

@hanthor-hive-agent

Finding

tests/pytest/test_save_perms.py tests files/kde-linux-system/save-perms/save-perms.py, which walks a build root and records non-default file modes plus every xattr os.listxattr reports (used to restore permissions/xattrs BuildStream would otherwise drop).

Three tests — test_regular_file_default_755_mode_omitted, test_regular_file_default_644_mode_omitted, test_retrieve_walks_tree — call retrieve_one/retrieve against a real temp file and assert the entry is omitted from the doc when the mode is default. They don't mock os.listxattr, so they implicitly assume a freshly created file reports zero xattrs. That's not guaranteed: on an SELinux-labeled filesystem, every new file automatically carries a security.selinux xattr, which save-perms.py records unconditionally (no filtering) — so the doc entry is non-empty and the "omitted" assertion fails, for a reason that has nothing to do with what the test is actually checking (mode handling).

This isn't hypothetical — it's directly reproducible:

$ python3 -c "import os,tempfile; print(os.listxattr(tempfile.mkstemp()[1]))"
['security.selinux']

Running the existing suite unmodified in an SELinux-labeled environment: 3 of 173 tests fail with exactly this AssertionError (e.g. 'etc/f' unexpectedly found in {'etc/f': {'attributes': {'security.selinux': '...'}}}). The module's own docstring says "os.chmod/os.setxattr are mocked so tests never depend on filesystem xattr support" — true for the apply_one tests, but not for these three retrieve-path tests, which is the gap.

The suite passes today because GH Actions' ubuntu-24.04 runners (.github/workflows/test.yml) don't set SELinux xattrs on new files — so this has been silently environment-dependent rather than actually deterministic.

There's also a real question about save-perms.py itself, not just the test: it copies security.selinux byte-for-byte into the permissions manifest and, on --restore, writes that value back with os.setxattr. A build/restore cycle running on an SELinux-enabled host would embed the build host's SELinux label into the image's build artifacts, not a label appropriate for the target system's own policy — worth a maintainer's look, separate from the test fix.

Evidence

  • Provenance: ran pytest tests/pytest/ -v --tb=short directly (commit at HEAD of main, 2026-09-03) in a container with SELinux-labeled tmpfs; reproduced independently via os.listxattr on a fresh mktemp file.
  • Filed as test-infrastructure, not coverage-gap: the tests exist and normally pass; the defect is that their correctness silently depends on an unstated environment property CI happens to satisfy by accident.

Recommendation

See companion PR — patches os.listxattr to return [] in the three affected tests, pinning the "no xattrs" precondition explicitly instead of relying on the host's ambient labeling, consistent with what the file's own docstring already claims the suite does. Verified: all 173 tests + 1 xfail pass, both with and without the SELinux xattr present.

Separately worth a maintainer's call: should save-perms.py filter security.selinux (and similar host-assigned labels) out of what it records, given the mismatch between "build host's kernel-assigned SELinux context" and "correct label for the shipped system"?

Priority

  • Impact: medium (any contributor or CI migration onto an SELinux-labeled runner — plausible for a Fedora-derived, bootc-targeting project — would see 3 tests fail with no clear connection to what they touched; the underlying save-perms.py question is unresolved but not confirmed as a shipping bug)
  • Effort: low (companion PR is the whole test-side fix)

Filed by quality agent (ACMM L4/L6 — full mode)

🐝 Hive Agent: quality | Instance: hive-keen-mink | SHA: unknown

— hive: agent=quality backend=claude model=claude-sonnet-5 claude=2.1.226

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/qualityApproved by a Hive merger/owner for auto-merge on green CIhive/hive-keen-minkApproved by a Hive merger/owner for auto-merge on green CIqualityApproved by a Hive merger/owner for auto-merge on green CItestingApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions