From 6d069038ba251b2b12a8b8d5c08461a1609b0d32 Mon Sep 17 00:00:00 2001 From: Derek Carr Date: Tue, 19 May 2026 11:33:41 -0400 Subject: [PATCH] feat(agents): add LSM compatibility checks to review and spike skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Teach the principal-engineer-reviewer agent, build-from-issue skill, and create-spike skill to assess Linux Security Module impact. Code that touches /proc filesystem access, process identity, or binary execution can behave differently on SELinux-enforcing or AppArmor hosts — for example, readlink on /proc//exe returns ENOENT across SELinux domain boundaries, breaking tests that fork+exec into system binaries. Signed-off-by: Derek Carr --- .agents/skills/build-from-issue/SKILL.md | 1 + .agents/skills/create-spike/SKILL.md | 4 ++- .claude/agents/principal-engineer-reviewer.md | 30 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/.agents/skills/build-from-issue/SKILL.md b/.agents/skills/build-from-issue/SKILL.md index dbb5396cd..1225e6248 100644 --- a/.agents/skills/build-from-issue/SKILL.md +++ b/.agents/skills/build-from-issue/SKILL.md @@ -148,6 +148,7 @@ In the prompt, instruct the reviewer to: - **Medium**: Multiple files/components, some design decisions, but well-scoped - **High**: Cross-cutting changes, architectural decisions needed, significant unknowns 8. Call out risks, unknowns, and decisions that need stakeholder input. +9. Assess **LSM compatibility** — if the change touches process identity, `/proc` filesystem access, binary execution, or inter-process visibility, flag whether it will behave differently on hosts running SELinux (enforcing) or AppArmor. In particular, tests that fork+exec into system binaries will fail on SELinux-enforcing hosts due to cross-label `/proc//exe` access restrictions. ### A2: Post the Plan Comment diff --git a/.agents/skills/create-spike/SKILL.md b/.agents/skills/create-spike/SKILL.md index faa7aca08..f141f82ef 100644 --- a/.agents/skills/create-spike/SKILL.md +++ b/.agents/skills/create-spike/SKILL.md @@ -91,7 +91,9 @@ The prompt to the reviewer **must** instruct it to: 9. **Check architecture docs** in the `architecture/` directory for relevant documentation about the affected subsystems. -10. **Determine the issue type:** `feat`, `fix`, `refactor`, `chore`, `perf`, or `docs`. +10. **Assess Linux Security Module (LSM) impact.** If the change involves process identity, `/proc` filesystem access, file labeling, binary execution, or inter-process visibility, call out whether it will behave differently on hosts running SELinux (enforcing) or AppArmor. For example: reading `/proc//exe` across an SELinux domain boundary returns ENOENT, not EACCES. Tests that fork+exec into system binaries (different SELinux label) will fail on enforcing hosts. Flag any LSM-sensitive code paths and recommend mitigations. + +11. **Determine the issue type:** `feat`, `fix`, `refactor`, `chore`, `perf`, or `docs`. ### What makes a good investigation prompt diff --git a/.claude/agents/principal-engineer-reviewer.md b/.claude/agents/principal-engineer-reviewer.md index ae7e49ea2..8badf491e 100644 --- a/.claude/agents/principal-engineer-reviewer.md +++ b/.claude/agents/principal-engineer-reviewer.md @@ -146,6 +146,36 @@ applies to every PR — use judgment. - **Supply chain:** Do new dependencies introduce known vulnerabilities or unmaintained transitive dependencies? +### Linux Security Module (LSM) compatibility + +OpenShell runs on hosts with SELinux or AppArmor in enforcing mode. +Review changes that interact with the `/proc` filesystem, process +identity, binary execution, or inter-process visibility for +LSM-related issues: + +- **`/proc//exe` across domain boundaries:** On SELinux-enforcing + hosts, readlink on `/proc//exe` returns ENOENT (not EACCES) when + the target process has a different SELinux label than the caller. + This affects any code that resolves binary identity after fork+exec + into a differently-labeled binary (e.g., system binaries under + `bin_t` vs. build artifacts under `user_home_t`). + +- **Tests that fork+exec into system binaries:** Tests that fork a child + and exec into `/bin/sleep`, `/usr/bin/cat`, or similar will fail on + SELinux-enforcing hosts because the child transitions to a different + domain, making its `/proc` entries unreadable to the parent. Flag + these tests and recommend either using a same-label helper binary or + skipping on enforcing hosts with a TODO. + +- **File labeling and Landlock interaction:** New files created in + non-standard paths may inherit unexpected SELinux labels. Verify that + Landlock and SELinux policies do not conflict. + +- **Socket and IPC visibility:** SELinux can restrict `/proc//fd` + and `/proc//net` visibility across domain boundaries. Code that + scans these paths for socket ownership should handle access failures + gracefully. + ## Principles - Don't nitpick style unless it harms readability. Trust `rustfmt` and the