fix(safety): block direct launchctl/systemctl service registration - #1311
Conversation
…1279) Add launchctl load/bootstrap/submit/start and systemctl enable/start/daemon-reload to all four safety guard layers: - risk-classifier: BASH_HIGH tier (triggers AFK approval prompt) - safe-destruct-patterns: BLOCK tier (hard block with redirect to /service-setup) - write-denylist: deny writes to ~/Library/LaunchAgents, LaunchDaemons, ~/.config/systemd - bash-restriction-hook: sensitive roots + SENSITIVE_PATH_SIGNAL regex Closes #1279
griffinwork40
added
bug
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9054ff9502
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| { | ||
| id: 'systemctl-service-enable', | ||
| re: /\bsystemctl\s+(enable|start|daemon-reload)\b/i, |
There was a problem hiding this comment.
The command form used by this repository is systemctl --user daemon-reload / systemctl --user enable --now (src/service/systemd/install.ts), but this regex requires the subcommand to immediately follow systemctl. I checked systemctl --help, which documents --user as the global option that connects to the user service manager; consequently, the valid persistent registration command systemctl --user enable --now attacker.service produces no safe-destruct match, and the identical literals in BASH_HIGH classify it only as medium. Account for global options before matching the blocked subcommands.
Useful? React with 👍 / 👎.
griffinwork40
left a comment
griffinwork40
left a comment
There was a problem hiding this comment.
🤖 Automated review (scheduled sweep), generated by the /review tool — a maintainer will follow up.
Prior Automated Round — Finding Dispositions
No prior automated rounds on this PR — this is the first automated review. No findings to carry forward.
Decision
Severity arithmetic: 2 high blocking (security/correctness: launchctl kickstart activation bypass; systemctl enable-linger double-block false-positive); 1 medium advisory (security: .config/systemd path mismatch in bash check-2); 2 medium advisory (test-coverage: no tests for 7 new BASH_HIGH entries; no tests for 2 new BLOCK patterns); 1 medium advisory (correctness: launchctl enable persistence bypass); 2 low advisory.
Per sweep policy: only critical and high block. medium and below are non-blocking follow-ups.
Review Summary
PR: fix(safety): block direct launchctl/systemctl service registration
Reviewed ref: 9054ff9502f256c3ab9e4a0e2bfa7c7be820c649
Regime: light (hotfix, ~50 lines added, 4 files)
CI: Lint & Build ✅, Test (ubuntu-latest) ✅, Test (macos-latest) ✅, all checks passing
The PR correctly adds launchd/systemd service-registration blocking across four guard layers. The architecture is sound and the stated mechanism is correctly implemented. Two high-severity defects remain that should be addressed before merge.
🔴 Blocking Findings (high)
H1 · high · blocking:true · security · src/agent/risk-classifier.ts · ref:9054ff9502 · diff-context
launchctl kickstart is absent from BASH_HIGH — service activation bypass.
afk service install (the sanctioned alternative the new blockReason text redirects agents to) internally calls launchctl kickstart -k gui/<uid>/<label> via Node execFileSync (file-state: src/service/launchd/manager.ts:96). Because Node execFileSync bypasses the bash tool hook, this is architecturally fine. However, launchctl kickstart is also the direct command an agent would use to immediately start a registered-but-not-yet-running service — and it is absent from BASH_HIGH and from the launchctl-service-register safe-destruct regex. An agent told "use afk service install" might instead discover launchctl kickstart and use it directly without triggering any guard.
# BASH_HIGH at reviewed ref — launchctl entries:
'launchctl load', 'launchctl bootstrap', 'launchctl submit', 'launchctl start'
# MISSING: 'launchctl kickstart'
# safe-destruct regex:
/\blaunchctl\s+(load|bootstrap|submit|start)\b/i
# MISSING: kickstart in alternation
Suggestion: Add 'launchctl kickstart' to BASH_HIGH and kickstart to the launchctl-service-register regex alternation: /\blaunchctl\s+(load|bootstrap|submit|start|kickstart)\b/i.
H2 · high · blocking:true · security/correctness · src/agent/safe-destruct-patterns.ts + src/agent/risk-classifier.ts · ref:9054ff9502 · diff-context
systemctl enable-linger false-positives on both the new safe-destruct BLOCK pattern and the BASH_HIGH substring entry.
Live verification:
'systemctl enable'.includes('systemctl enable')when applied to'systemctl enable-linger root'→true(BASH_HIGH raw substring match, line 191 of risk-classifier.ts)/\bsystemctl\s+(enable|start|daemon-reload)\b/i.test('systemctl enable-linger root')→true(word boundary\bfires betweenenableand-)
The correct command for session-level login persistence is loginctl enable-linger (the sanctioned path documented at src/service/systemd/install.ts:211). A user who accidentally types systemctl enable-linger — a plausible typo — triggers a hard BLOCK with a misleading message ("enables/starts a systemd unit that persists across sessions") when the actual command was a user-level linger toggle, not a service installation. This is the same regex false-positive identified in prior reviews of the companion PR (#1298).
// Live regex test — CONFIRMED false-positive:
/\bsystemctl\s+(enable|start|daemon-reload)\b/i.test('systemctl enable-linger root') // → true
// BASH_HIGH substring — CONFIRMED false-positive:
'systemctl enable-linger root'.includes('systemctl enable') // → trueSuggestion (safe-destruct): Replace terminal \b with (?:\s|$) in the systemctl-service-enable regex:
re: /\bsystemctl\s+(enable|start|daemon-reload)(?:\s|$)/i,
Suggestion (risk-classifier): Append trailing space: 'systemctl enable ' (so enable-linger does not match).
🟡 Advisory Findings (non-blocking — follow-up issues recommended)
M1 · medium · advisory · security · src/agent/tools/hooks/bash-restriction-hook.ts · ref:9054ff9502 · diff-context
.config/systemd path mismatch: write-denylist blocks the full tree; bash check-2 only blocks user/ subdir.
The write-denylist adds ${homedir()}/.config/systemd (entire tree). The bash-restriction-hook adds path.join(home, '.config', 'systemd', 'user') (user subdir only). Check-2 (literal path substring scan in deriveRestrictedSubstrings) therefore does not block cat ~/.config/systemd/system/evil.service — the user suffix causes a non-match. The SENSITIVE_PATH_SIGNAL regex covers .config/systemd (no /user suffix) correctly, so check-1 (interpreter eval guard) does catch it. Live test: ~/.config/systemd/system/evil.service — check-2 miss, SENSITIVE_PATH_SIGNAL hit.
Suggestion: Add path.join(home, '.config', 'systemd') (without /user) to builtinBashSensitiveRoots() to align with the write-denylist entry. The more specific user/ subdir entry can remain alongside it.
M2 · medium · advisory · test-coverage · src/agent/risk-classifier.test.ts · ref:9054ff9502 · absence confirmed
0 tests for the 7 new BASH_HIGH entries in risk-classifier.test.ts.
Searched risk-classifier.test.ts at reviewed ref for launchctl and systemctl: 0 matches. Every prior BASH_HIGH addition has dedicated test coverage; the 7 new entries have none. A silent deletion of any of them would pass the full test suite.
Suggestion: Add a describe block with it() for each pattern: launchctl load, launchctl bootstrap, launchctl submit, launchctl start, systemctl enable, systemctl start, systemctl daemon-reload.
M3 · medium · advisory · test-coverage · src/agent/safe-destruct-detect.test.ts · ref:9054ff9502 · absence confirmed
0 tests for the 2 new BLOCK patterns in safe-destruct-detect.test.ts.
Searched safe-destruct-detect.test.ts at reviewed ref for launchctl, systemctl, launchctl-service-register, systemctl-service-enable: 0 matches. The BLOCK patterns (launchctl-service-register, systemctl-service-enable) — the core fix for the 2026-08-24 incident — have no regression guard. A future edit that misconfigures the pattern id, tier, or regex would go undetected.
Suggestion: Add to the BLOCK it.each table:
launchctl bootstrap gui/501 /path/to.plist→'launchctl-service-register'systemctl enable afk-telegram.service→'systemctl-service-enable'systemctl daemon-reload→'systemctl-service-enable'
M4 · medium · advisory · correctness · src/agent/risk-classifier.ts · ref:9054ff9502 · diff-context
launchctl enable (persistence-without-load) is not blocked.
launchctl enable <service> marks a service as enabled in the launchd disabled-database — it persists across reboots and causes the service to start at next boot WITHOUT immediately loading it. It is absent from BASH_HIGH and from the launchctl-service-register safe-destruct regex. Less severe than bootstrap/load (no immediate execution), but it is a persistent system modification that survives reboots.
Suggestion: Add 'launchctl enable' to BASH_HIGH and enable to the launchctl-service-register regex alternation (if H1 is addressed: /\blaunchctl\s+(load|bootstrap|submit|start|kickstart|enable)\b/i).
L1 · low · advisory · spec-compliance · diff-context
PR title says "service registration" but start/daemon-reload are activation/housekeeping commands, not registration. No code change needed — cosmetic doc gap only. Update the commit body to say "registration and activation commands" to accurately describe the block scope.
L2 · low · advisory · test-coverage · src/agent/tools/handlers/write-denylist.test.ts · ref:9054ff9502 · file-state
Regression guard list (describe('regression guard — all original BUILTIN_WRITE_DENYLIST entries still block')) does not enumerate the 5 new entries. The meta-test at write-denylist.test.ts:97-103 iterates BUILTIN_WRITE_DENYLIST and provides automatic coverage for new entries. But the static regression guard at line 339 (which explicitly lists the original 8 entries) will not catch a future deletion of the new entries. Low severity — the meta-test covers the gap.
Spec-compliance
Stated intent: PR #1311 title+body — spec-compliance assessed.
| Requirement | Status |
|---|---|
BASH_HIGH: launchctl load/bootstrap/submit/start |
✅ All 4 added |
BASH_HIGH: systemctl enable/start/daemon-reload |
✅ All 3 added; systemctl enable has false-positive on enable-linger (H2) |
safe-destruct: launchctl-service-register BLOCK pattern |
✅ Implemented; kickstart missing (H1) |
safe-destruct: systemctl-service-enable BLOCK pattern |
✅ Implemented; enable-linger false-positive (H2) |
write-denylist: LaunchAgents + LaunchDaemons (user + system) |
✅ 4 entries added |
write-denylist: .config/systemd |
✅ Added |
bash-restriction-hook: builtinBashSensitiveRoots() |
✅ 5 entries added |
bash-restriction-hook: SENSITIVE_PATH_SIGNAL |
✅ 3 fragments added |
| All 253 existing tests pass | ✅ Stated in PR body; CI green |
Dimensions With No Issues
- API-compat — no issues found. All changes are purely additive (new
BASH_HIGHentries, newDESTRUCTIVE_PATTERNSentries, newBUILTIN_WRITE_DENYLISTentries, newbuiltinBashSensitiveRoots()entries, extended regex). No exported type signatures changed. No breaking changes. - Perf-observability — no issues found. 7 new entries in a small linear-scan array, 2 new compiled regex tests per bash call, 5 new path entries — all negligible additions to existing O(n) scans. No hot-path regressions.
What Was Not Checked
- Citations verified inline against branch HEAD
9054ff9502f256c3ab9e4a0e2bfa7c7be820c649. - H1 (launchctl kickstart) shadow-verified:
manager.ts:96confirms kickstart is the restart path;risk-classifier.ts:95-104confirms kickstart is absent. - H2 (enable-linger false-positive) shadow-verified: live regex test at reviewed ref confirms both the BLOCK pattern and the BASH_HIGH substring match
systemctl enable-linger root. - Stated intent: PR #1311 title+body — spec-compliance assessed.
- Not checked: runtime test execution — static review only. CI shows all checks passing.
- Not checked: whether
afk service installcorrectly bypasses all four layers at runtime (confirmed architecturally: it uses NodeexecFileSync, not the bash tool). - Not checked: Windows platform behavior (no Windows-specific changes in this PR).
griffinwork40
left a comment
griffinwork40
left a comment
There was a problem hiding this comment.
🤖 Automated review (scheduled sweep), generated by the /review tool — a maintainer will follow up.
Prior Automated Round — Finding Dispositions
1 prior automated review (on commit 9054ff9502, 2026-08-27). Current HEAD is the same commit — no new code since the prior round. All findings are re-confirmed at the same ref.
| Prior finding | Disposition |
|---|---|
H1 (high): launchctl kickstart absent from BASH_HIGH and safe-destruct regex — service activation bypass |
STILL-PRESENT — no change at 9054ff9502; kickstart remains absent from both BASH_HIGH and the launchctl-service-register alternation. |
H2 (high): systemctl enable-linger false-positives on both safe-destruct BLOCK pattern and BASH_HIGH substring match |
STILL-PRESENT — no change; \b terminal word boundary still fires between enable and - in enable-linger; 'systemctl enable'.includes() still matches systemctl enable-linger. |
M1 (medium): .config/systemd path mismatch — write-denylist blocks full tree; bash check-2 only blocks user/ subdir |
STILL-PRESENT — advisory, non-blocking. |
M2 (medium): 0 tests for the 7 new BASH_HIGH entries |
STILL-PRESENT — advisory, non-blocking. |
| M3 (medium): 0 tests for the 2 new BLOCK patterns | STILL-PRESENT — advisory, non-blocking. |
M4 (medium): launchctl enable (persistence without load) not blocked |
STILL-PRESENT — advisory, non-blocking. |
| L1 (low): PR title says "registration" but also blocks activation commands | STILL-PRESENT — cosmetic, non-blocking. |
| L2 (low): Regression guard list in write-denylist.test.ts doesn't enumerate new entries | STILL-PRESENT — non-blocking. |
Decision
Severity arithmetic: 2 high blocking (security: launchctl kickstart bypass, H1; correctness: systemctl enable-linger false-positive across two layers, H2); 4 medium advisory (non-blocking); 2 low advisory (non-blocking).
Per sweep policy: only critical and high findings block. The 4 medium and 2 low are non-blocking follow-ups.
Review Summary
PR: fix(safety): block direct launchctl/systemctl service registration
Reviewed ref: 9054ff9502f256c3ab9e4a0e2bfa7c7be820c649 (unchanged since prior round)
Regime: light (hotfix, ~50 lines added, 4 files)
CI: Lint & Build ✅, Test (ubuntu-latest, macos-latest) ✅, all checks passing
No new commits since the prior automated round. The two high-severity findings from that round remain unfixed. The architecture is sound and the stated mechanism is correctly implemented for the verbs it covers — but the two gaps (missing kickstart verb, enable-linger false-positive) remain open.
🔴 Blocking Findings (high) — carried forward, still present
H1 · high · blocking:true · security · src/agent/risk-classifier.ts + src/agent/safe-destruct-patterns.ts · ref:9054ff9502
launchctl kickstart absent from BASH_HIGH and safe-destruct regex — service activation bypass.
launchctl kickstart is the direct command to immediately start a registered-but-not-yet-running service. It is absent from both BASH_HIGH and the launchctl-service-register safe-destruct regex alternation. An agent told "use afk service install" might discover and use launchctl kickstart directly without triggering any guard.
Suggestion: Add 'launchctl kickstart' to BASH_HIGH and kickstart to the regex alternation: /\blaunchctl\s+(load|bootstrap|submit|start|kickstart)\b/i.
H2 · high · blocking:true · security/correctness · src/agent/safe-destruct-patterns.ts + src/agent/risk-classifier.ts · ref:9054ff9502
systemctl enable-linger false-positives across TWO independent security layers.
Layer 1 (safe-destruct): The regex /\bsystemctl\s+(enable|start|daemon-reload)\b/i false-positives on systemctl enable-linger because \b fires between enable and -.
Layer 2 (risk-classifier): The BASH_HIGH substring 'systemctl enable' matches via .includes(): 'systemctl enable-linger'.includes('systemctl enable') → true.
The correct command is loginctl enable-linger. A user who runs systemctl enable-linger hits a double-layer false block.
Suggestion (safe-destruct): Replace terminal \b with (?:\s|$):
re: /\bsystemctl\s+(enable|start|daemon-reload)(?:\s|$)/i,
Suggestion (risk-classifier): Append trailing space: 'systemctl enable '.
🟡 Advisory Findings (non-blocking — follow-up issues recommended)
- M1 (medium):
.config/systemdpath mismatch between write-denylist (full tree) and bash-restriction-hook check-2 (user/subdir only). - M2 (medium): 0 tests for the 7 new
BASH_HIGHentries inrisk-classifier.test.ts. - M3 (medium): 0 tests for the 2 new BLOCK patterns in
safe-destruct-detect.test.ts. - M4 (medium):
launchctl enable(persistence-without-load) not blocked. - L1 (low): PR title scope mismatch (cosmetic).
- L2 (low): Regression guard list doesn't enumerate new write-denylist entries.
What Was Not Checked
- Citations verified inline against branch HEAD
9054ff9502f256c3ab9e4a0e2bfa7c7be820c649. - Stated intent: PR #1311 title+body — spec-compliance assessed.
- Not checked: runtime test execution — static review only. CI shows all checks passing.
- Not checked: whether
afk service installcorrectly bypasses all four layers at runtime.
…able-linger false-positive, systemctl flags, path alignment, tests
Adopt the same flag-prefix pattern used by git-reset-hard and
git-clean-force — (?:(?:-{2}[\w-]+(?:=\S+)?|-[A-Za-z]…)\s+)* — so
that systemctl --type=service enable and similar --key=val forms
are caught by the safe-destruct BLOCK regex.
Adds a test case for the --type=service form.
|
Finding 3 re-evaluated: Now Wave 2 - Synthesis: PR #1311 Review —
|
…consistency - Add 4 missing launchctl verb tests (load, start, submit, enable) to safe-destruct-detect BLOCK table; incident-vector `launchctl load` was untested at Layer 2 - Add trailing space to `'launchctl start '` in BASH_HIGH for consistency with `'launchctl enable '` and `'systemctl start '` - Add false-positive negative tests for `enable-linger` at Layer 1 (risk-classifier) matching the existing Layer 2 coverage
1 participant
Summary
Closes #1279 — blocks direct
launchctl/systemctlservice registration commands across all four safety guard layers so agents must go throughafk service install(the/service-setupskill).This was the primary enabler of the 2026-08-24 incident where an agent autonomously loaded a launchd plist that posted to a production Substack account without human approval.
Changes
BASH_HIGHrisk classifierrisk-classifier.tslaunchctl load/bootstrap/submit/start,systemctl enable/start/daemon-reloadsafe-destruct-patternssafe-destruct-patterns.ts/service-setupwrite-denylist.ts~/Library/LaunchAgents,~/Library/LaunchDaemons,/Library/Launch*,~/.config/systemdbash-restriction-hook.tsbuiltinBashSensitiveRoots()+SENSITIVE_PATH_SIGNALregexTesting
SENSITIVE_PATH_SIGNALsync test now covers the new pathspnpm lintclean