fix(service): refuse a system unit SELinux proves cannot start

[chenmingwei23]

What is the problem?

On an SELinux-enforcing host whose kirocrew lives under $HOME -- the default on
Bazzite, Fedora Silverblue/Kinoite and other atomic desktops -- kirocrew service install writes a system unit at /etc/systemd/system/kirocrew.service that
can never start. It fails every start with status=203/EXEC and crash-loops
until it exhausts StartLimitBurst.

203/EXEC is systemd reporting that it could not execute the binary, and it has
four causes that are indistinguishable in the unit's status output: the path
does not exist, it is not executable, its shebang interpreter is wrong, or the
file is perfectly fine and SELinux refused the execute. This issue is the last
one, which is the only one that survives every check an installer would normally
run.

I established which cause it is before changing anything. The evidence is a
read-only compute_av query against the loaded policy on a Fedora-family
targeted policy (/sys/fs/selinux/access -- a pure decision lookup that
relabels nothing, needs no root, and writes no state):

source domain	target label	execute	getattr
system_u:system_r:init_t:s0 (PID 1)	user_home_t	DENY	ALLOW
system_u:system_r:init_t:s0 (PID 1)	bin_t	ALLOW	ALLOW
unconfined_u:unconfined_r:unconfined_t (user manager)	user_home_t	ALLOW	ALLOW

The reply's flags word was 0, so init_t is not a per-domain permissive
domain -- this is a real policy denial, not one the kernel would log and allow.

getattr ALLOW with execute DENY is the whole diagnostic problem. It means
a positive on "is this path fine?" is exactly what you get in the broken case:
the file exists, os.access(..., X_OK) and test -x return True, stat
works, the shebang is correct. A file that passes test -x and still fails
203/EXEC is the signature of this cause, and no file-mode check can distinguish
it. Only asking the policy can.

Why this issue matters to the user

Out-of-the-box kirocrew service install is broken on every atomic Fedora
variant, and it fails in the least useful way available: the install writes the
unit, enables it, then fails at the first systemctl restart with "run
journalctl for details". The host is left with an enabled unit that
crash-loops at every boot, and the message names none of the cause. The
reporter had to read the audit log themselves to find it.

How our fix solves it

Chain from symptom to root cause: the symptom is a crash-looping unit; the
mechanism is init_t denied execute on the ExecStart binary's label; the root
cause is that the installer commits to system scope without ever asking whether
system scope can work on this host. So it now asks, before it writes anything.

New kiro_crew/service/selinux.py answers one question -- can the domain that
will perform the execve actually execute the file we are about to name in
ExecStart? -- and install() refuses up front when the answer is provably no.

Three properties matter:

• Mechanism-based, never distro-based, the same rule apparmor.py already
  states for its own gate. Nothing matches a distro name, a version, or even a
  type name. Every input is read from the running kernel: enforcing state from
  /sys/fs/selinux/enforce, the executing domain from /proc/1/attr/current
  (whatever PID 1 actually is, not a hardcoded init_t), the file's label from
  its security.selinux xattr, and the verdict from the loaded policy. This is
  not theoretical: on my development host the checkout is outside /home, so it
  is labelled default_t rather than the user_home_t the issue reports -- and
  the gate fired correctly on it. A type-name match would have missed it.
• Fires only on a proven positive; fails OPEN everywhere else. SELinux absent,
  permissive mode, an unreadable label, a kernel that refuses the query, a
  truncated reply, a per-domain permissive source -- every one returns "not
  blocked" and the install proceeds byte-identically to today. A pre-flight that
  guesses wrong in the refusing direction would break installs that currently
  work, which is worse than the bug.
• Refusal, not a warning. Everything after that point is destructive to no
  purpose. Stopping before the first write leaves the host exactly as found,
  instead of enabled-and-crash-looping.

The refusal embeds a ready-to-paste user-scope unit rendered by the real
render_unit(), so the operator's working unit carries the same ExecStart and
the same baked environment as the unit we would have installed and cannot drift
from it. Two directives differ, and both are hard requirements rather than style:
User=/Group= are omitted (a user manager rejects them, making the unit
unloadable) and WantedBy=default.target replaces multi-user.target (a system
target the user manager does not have). render_unit()'s default is unchanged,
so every existing caller gets today's unit byte-for-byte.

The message also records a fix that looks right and provably is not:
relocating only the launcher to a system-labelled path does not help, because
whatever systemd execs still runs in PID 1's domain, so the next execve of the
binary under $HOME is denied identically. init_t -> user_home_t is denied
regardless of who initiates the chain.

No override knob, deliberately. The check reads the live policy, so an
operator who loads a policy module granting the access turns it off by itself --
compute_av starts answering ALLOW and the gate goes quiet with no flag to
remember. An escape hatch could only ever re-enable an install that provably
crash-loops.

What this PR deliberately does NOT do

It does not add a --user install scope (option 1 in the issue). That is an
install-model change, not a mechanical one: it has to decide where the AppArmor
profile lives when the install needs no root, what replaces the root-owned
/etc/kirocrew/kirocrew.env, how status/restart/uninstall/logs become
scope-aware, and how Make Live's drop-in indirection (#1598) follows. Shipping
half of that would add a second broken path. The predicate this PR lands is the
groundwork an auto-select would call, and the issue stays open for that decision.

What tests we did

50 new tests in test/test_service_selinux.py, all pure-logic: every kernel
interface is redirected at a tmp_path fake or monkeypatched, so the suite gives
identical verdicts on an AppArmor-only CI runner and on an SELinux-enforcing
workstation, and no test is ever in a position to write to real selinuxfs. That
constraint drove one design change -- the selinuxfs transport was split into
_query_access() so tests patch a module seam instead of os.open/os.read
globally, which breaks pytest's own I/O and leaves a test one edit away from
writing to /sys.

Mutation-verified: 17/17 mutants caught. Each guard was broken in turn and a
named test had to fail:

mutant	caught by
enforcing check removed	test_quiet_when_not_enforcing
per-domain permissive flag ignored	test_quiet_when_source_domain_is_permissive
perm bit used as a mask without shifting	test_bit_number_becomes_a_mask
flags read from the seqno field	test_permissive_flag_is_taken_from_the_last_field
symlink not resolved before reading the label	test_label_is_read_through_a_symlink_chain
shebang interpreter never checked	test_fires_on_a_denied_shebang_interpreter
gate warns instead of refusing	test_install_raises_before_touching_the_host
user unit keeps User=/Group=	test_user_unit_omits_user_and_group
user unit keeps multi-user.target	test_user_unit_wants_default_target
gate asked about the wrong path	test_gate_is_asked_about_the_unit_exec_path
remedy reverts to a tilde home	test_refusal_never_lets_the_pasting_shell_pick_the_account
remedy reverts to $USER for linger	test_refusal_never_lets_the_pasting_shell_pick_the_account
remedy drops the run-as-root warning	test_refusal_warns_that_a_root_shell_would_run_the_agent_as_root
remedy paths unquoted (space-bearing home)	test_remedy_quotes_a_home_containing_a_space
start-failure hint never fires	test_enforcing_host_gets_the_selinux_hypothesis_and_remedy
start-failure hint fires on every host	test_non_enforcing_host_gets_no_selinux_noise
restart failure drops the hint	test_enforcing_host_gets_the_selinux_hypothesis_and_remedy

End-to-end against this host's real kernel, in two arms. Arm A: untouched.
The host is SELinux-enabled but permissive, so the gate must stay silent -- it
did, via the real /sys, exercising the fail-open path. Arm B: only
selinux_is_enforcing forced True, every other input real -- the real
/proc/1/attr/current (system_u:system_r:init_t:s0), the real
security.selinux xattr of the real installed binary, and a real compute_av
query against the real loaded policy. It fired and named the real label. That is
the reporter's host in every respect except the one bit this machine has set
differently.

test/test_service.py (301 tests) passes unchanged. Four failures in it are
pre-existing and unrelated -- TestExpectedUidOverride / TestATakeoverOf...
fail with "the directory /local/home is owned by uid 65534", the single-uid
user-namespace mapping in my tool sandbox; verified failing identically on the
base commit before my changes. flake8, isort and mypy are clean on all
changed files.

Review round 1

GPT 5.6 raised one blocking, security-class finding and it was correct, so it is
fixed rather than overridden. A user unit carries no User= -- the account it runs
as is whichever manager loads it -- and service install is documented to run
under sudo, so the shell reading this refusal is usually root's. My first
remedy said mkdir -p ~/.config/systemd/user and loginctl enable-linger "$USER", both resolved by the pasting shell: from root they name /root and
root, and the operator ends up running untrusted agent tools as root --
precisely the invariant install() enforces by refusing a User=root unit a few
lines earlier. The remedy now spells out the resolved account and its absolute
home, passes the account name to loginctl explicitly, says what a root shell
would do, and notes that sudo -u <user> cannot substitute (it creates no
session, so systemctl --user has no manager to reach). Three mutants pin it.

The first-principles review was advisory (CONCERNS) and both of its subtractions
were taken: render_unit's two-value scope string became user_scope: bool
(exactly one non-default variant is ever constructed) and the four selinux.py
helpers are underscore-prefixed, leaving blocks_system_unit the module's only
public surface. Design Review PASS and Opus (no findings) needed no changes.

Review round 2

GPT raised a second blocking finding and it is also real, though its stated
symptom is not: a KIROCREW_SERVICE_BIN override can name a system-labelled
wrapper that goes on to run a binary under $HOME. The pre-flight sees only the
wrapper and its shebang interpreter, both allowed, so it reports "not blocked" and
the unit still cannot serve. It does not, however, fail with 203/EXEC -- the
wrapper execs fine, so the inner denial surfaces as the shell's exit 126. The
defect is real even though the mechanism was described wrong, so it is fixed
rather than overridden.

It is fixed at the point of failure rather than by closing the static hole,
because the hole cannot be closed soundly. Following the delegation means deciding
what an arbitrary shell script executes. The weaker version -- scanning the wrapper
for path-shaped literals -- would refuse an install over a path in a comment or an
untaken branch, since a literal appearing in a script is not proof it is ever
executed; and refusing on "cannot inspect" is the same violation from the other
side. Either would manufacture false refusals and break the fail-open rule the
whole module rests on.

So: the pre-flight's coverage boundary is now stated explicitly in the module
docstring, and install() covers the residue where the unit has actually failed.
When the first systemctl restart fails on an enforcing host, the error now names
SELinux as a candidate cause, says plainly that the pre-flight found no proven
denial and why its reach is limited, gives ausearch -m avc -ts recent to confirm
it, and prints the same user-scope remedy. On a non-enforcing host the message is
unchanged. That covers wrappers, computed paths, and anything else static analysis
cannot prove, and it adds no false refusals -- five mutants pin it.

The non-blocking finding was also correct and is fixed: both generated paths in the
remedy now go through shlex.quote, matching what common.py already does for the
.env remedy, so an account home containing a space no longer word-splits mkdir
and land the unit where systemd never reads it.

Review round 3 (CI only, no new review findings)

Three CI reds, all mine, all fixed: the De-Amazon scrub lint reads /home/two words (my space-bearing test fixture) as a personal home path, so the fixture is
now /home/tester with space, whose first segment matches the placeholder the rest
of the file already uses -- the lint passes locally. The Windows shard failed
test_ordinary_paths_are_not_needlessly_quoted, because shlex.quote is POSIX and
quotes the backslashes in a Windows path, so "needs no quoting" is only meaningful
on POSIX; that one test is now POSIX-scoped with the reason stated. The branch is
also rebased onto current main and squashed to one commit, which the Hygiene gate
requires (it caps a PR at two).

One remaining red is not mine, and I proved it rather than asserting it.
Backend Tests (Windows) (2) fails
test_dashboard_cron_to_chat.py::...::test_a_repeat_is_suppressed_even_though_history_is_none
with assert 1 == 2. Nothing in this diff reaches the cron-to-chat path, so I
traced the mechanism instead of waving at it. CronJob.set_run_result() stamps
last_result_ts = time.time(); cron_inject renders the run-boundary marker as
<!-- ...<job.id>:<ts:.6f} --> and the dedupe compares the whole row, so that
stamp is what makes two runs distinguishable. The test does two runs back to back
with no sleep. On Linux time.time() advances at microsecond scale and the markers
differ; on Windows under CPython 3.12 it comes from GetSystemTimeAsFileTime at
the ~15.625 ms system tick, both calls return the same float, the second row is
byte-identical, and it is dropped as a repeat. The CI Windows shard pins 3.12 on
purpose, and time.time()'s Windows resolution only improved in 3.13.

Confirmed by reproducing it on Linux: quantizing time.time() to a 15.625 ms tick
makes the identical assertion fail deterministically, with none of this change
involved; the same test passes on the same tree with the real clock. It presents as
intermittent because whether the two calls straddle a tick depends on load and on
what ran before them, which is why merely adding tests to the suite can flip it.
Filed separately as #8502 with the mechanism and the repro rather than folded in
here, and the lane is being re-run.

Review round 4

Board on the current head: Design Review PASS, Opus 4.8 no findings, GPT 5.6 no
findings and no [BLOCK-MERGE]. First Principles is CONCERNS (advisory), and one of
its two subtractions was a real defect I had shipped, so it is taken.

Taken. The start-failure hint pasted the full 25-line user-scope remedy onto
every failed systemctl restart on any enforcing host -- which is all of
RHEL/Fedora -- including hosts the pre-flight had positively proved ALLOW for. A
plain port conflict on a stock RHEL box would have been answered with a wall of
SELinux text and a pasteable unit for a denial nobody had observed, and the
function's own docstring conceded the denial is a hypothesis there. The hint is now
the hypothesis, the ausearch command that settles it, and a pointer to the
documented remedy: 15 lines instead of 68, with the full remedy reachable only
behind a proven denial. Two tests pin it -- one asserts the remedy is absent from
the hint path, one bounds the hint's size and its ratio to the refusal.

Declined, with reasoning. The second subtraction asks to drop
blocks_system_unit's not-blocked reason string as having zero consumers. It is
true that install() reads the reason only inside if blocked:. But that string is
what lets a test say WHICH fail-open branch fired -- test_quiet_when_not_enforcing
asserts "not enforcing" appears, rather than merely that the call returned False.
In a gate whose correctness is mostly its five distinct fail-open branches (not
enforcing, unreadable PID 1 context, no label, no policy answer, permissive domain),
collapsing them to a bare False removes the only evidence that the intended branch
is the one being exercised, and a test that cannot tell them apart would pass with
four of them wired to the wrong condition. Keeping ~40 bytes of reason string is
cheaper than losing that discrimination.

What I could NOT verify, stated plainly

I did not install, enable, start, or modify any systemd unit, and did not run
restorecon, chcon, semanage or setenforce. This is a shared development
host with other work running on it, and changing its SELinux state or its units
would affect all of it. Concretely unverified:

1. A real 203/EXEC reproduction. I did not install a system unit and watch
   it fail. The causal claim rests on the policy verdict plus systemd's
   documented behaviour when execve returns EACCES, not on an observed crash
   loop.
2. Enforcing mode. My host is permissive, so I forced the enforcing bit in
   Arm B rather than observing it. compute_av returns the policy decision
   independent of global mode, and flags=0 rules out per-domain permissive, so
   the verdict is real -- but the consequence of that verdict is inferred.
3. Bazzite's exact policy. My host runs Amazon Linux 2023's targeted
   policy, Fedora-derived and the same policy name, not Bazzite's Fedora 44
   policy. init_t-denied-execute-on-home-labels is core refpolicy and the
   reporter's own AVC matches it, but I confirmed it on a sibling policy, not
   theirs.
4. The wrapper case end to end. The start-failure hint is unit-tested, but I
   did not install a wrapper unit and watch it fail with exit 126, for the same
   host-safety reason as (1).
5. That the printed user unit starts. I did not load it. Its scope-correctness
   is reasoned from systemd.unit(5) plus the reporter's own verified workaround,
   and I did confirm locally that systemd --user runs in unconfined_t (read
   from /proc/<pid>/attr/current on two live user managers) and that
   unconfined_t is allowed execute on user_home_t -- which is the structural
   reason a user unit is not affected.

The honest summary: the cause is directly measured, the remedy's mechanism
is directly measured, and the end-to-end behaviour on an enforcing atomic
desktop is reasoned, not observed. A maintainer with a Bazzite or Silverblue
host can close gaps 1-4 in about five minutes.

Any other suggestions on the work

• kirocrew doctor should report this too, so an operator who already has a
  broken unit installed learns why without re-running install. Left out to keep
  this diff to one concern.
• The same question exists for pods (kirocrew-pod@.service is a user unit, so it
  is unaffected) and for the AppImage launcher path; neither is touched here.
• If a --user scope does land, the auto-select heuristic the issue asks for is
  exactly selinux.blocks_system_unit(kirocrew_bin()) -- the predicate is already
  in the shape a caller would want.

Pattern harvest

Rule candidate: review-checklist
Pattern: an execute-permission conclusion drawn from os.access(path, os.X_OK) or test -x. Under an LSM execute denial both return True while execve fails, so the check cannot fail in the failing case. Any code that gates an exec decision on a mode test needs a second question: is the caller's domain permitted to execute it?

Rule candidate: semgrep
Pattern: a test that monkeypatches os.open / os.read / os.write to fake a /sys or /proc interface. It breaks pytest's own I/O and leaves the test one edit away from performing the real write; the fix is to extract a module-level transport seam and patch that.

Supporting notes

• 203/EXEC is four different bugs wearing one error code. Before fixing an
  exec failure, name which of "absent / not executable / bad interpreter / policy
  denied" it is. They are indistinguishable in systemctl status and have
  disjoint fixes.
• State what a positive would have looked like before trusting one. Here
  getattr is ALLOW while execute is DENY, so "the path exists and is
  executable" returns True in precisely the broken case. This generalises past
  SELinux: whenever a denial and a success share an observable, find the
  observable that separates them.
• A policy question has a read-only oracle; use it instead of an experiment.
  /sys/fs/selinux/access answers "would this be allowed?" without performing
  the action, needing root, or changing any state. Reaching for ausearch after
  a crash-loop is the same answer obtained destructively and later.
• Gate on the mechanism, never the distro (inherited from apparmor.py, and
  it paid off again). The gate fired on default_t on my host and user_home_t
  on the reporter's; any type-name or distro match would have missed one of them.
• A diagnostic that can refuse must fail open on every indeterminate answer.
  Enumerate the "cannot tell" branches explicitly -- absent interface, unreadable
  label, refused query, truncated reply, permissive domain -- and route them all
  to "proceed".
• Prove a pre-existing failure on the base commit before attributing it to the
  environment. The four uid 65534 failures were confirmed identical on base;
  saying "sandbox artifact" without that check is a guess.
• Never print a command you have not confirmed ships. My first draft of the
  refusal told operators to run kirocrew service print-unit, which does not
  exist -- the subcommand list is install/uninstall/status. There is now a
  test asserting the message names only real subcommands, because a remedy that
  sends the user to invalid choice is worse than no remedy.

Fixes #7165

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Fail-open, mechanism-derived pre-flight that converts a silent crash-loop into an up-front refusal, with the real --user scope correctly deferred.

[DESIGN-REVIEWED] 2e81647

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

Every item traces to the kernel-verified defect or a hole this change itself opens, the deeper --user fix is declared and deferred, and nothing duplicates an existing mechanism.

What this change ships

Intent: stop kirocrew service install from enabling a system unit that SELinux provably prevents from ever starting — a FIX.

1. Install now refuses up front on a proven execve denial — justified (the fix, cause-level per its own chain)
2. New pre-flight that asks the loaded policy directly — justified (grepped selinux in src/: 5 files, no existing pre-flight; mirrors apparmor.should_install()'s documented (bool, reason) gate rather than duplicating it)
3. Refusal embeds a rendered, pasteable per-user unit — justified (paths/account are per-host, so static docs cannot carry it)
4. render_unit(user_scope=) — justified; 1 production consumer passes True (linux.py:554), but the other passes the default, so not a single-value generalization
5. Restart failures on any enforcing host now append an SELinux hypothesis + ausearch command — declared in code, not in the (truncated) description text; derived from the gate's own coverage hole, since KIROCREW_SERVICE_BIN is documented for wrapper scripts (service/common.py:80)
6. Docs section in install.md — mandated (spec-in-same-commit invariant)
7. --user install scope deliberately not shipped — declared, deferred to the open issue with the level named, exactly what root-cause accounting requires

Watch

Item 5 fires on every failed restart on all of RHEL/Fedora, including hosts the gate proved ALLOW for; the author bounded it (hypothesis-only, no remedy paste, empty when not enforcing) and pinned the bound in tests, so it earns its place — but it is the one item a human should confirm the full description actually declares.

[FIRST-PRINCIPLES-REVIEWED] 2e81647

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 2e81647

Verdict parsed from the review's SHA-scoped output markers for commit 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2e81647

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision 2e8164769c1bbdfcfb2f07ed3bcc9c5d416eaf91 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[bolichen97]

Reviewed the full diff. blocks_system_unit refuses only when enforce=1, both contexts resolve, compute_av parses, permissive is clear and execute is absent on the realpath'd binary/interpreter -- every indeterminate branch returns False, so non-SELinux Linux and macOS are byte-identical. No subprocess added; 753/1308 lines are tests. Two Low notes: two stale docstrings in selinux.py/linux.py, and Fixes #7165 contradicts the body's "issue stays open" -- please drop or reword the trailer. Approving.