fix: Safe rendering of sandbox violation messages - #391
Conversation
SafeDep Report SummaryNo dependency changes detected. Nothing to scan. This report is generated by SafeDep Github App |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
+ Coverage 58.66% 58.81% +0.15%
==========================================
Files 222 222
Lines 15276 15352 +76
==========================================
+ Hits 8961 9029 +68
- Misses 5501 5507 +6
- Partials 814 816 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a23717890d
ℹ️ 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".
| safe := *violation | ||
| safe.Target = quoteTerminalText(safe.Target) | ||
| safe.Process = quoteTerminalText(safe.Process) | ||
| safe.RuleLabel = quoteTerminalText(safe.RuleLabel) |
There was a problem hiding this comment.
Escape the raw audit log before terminal rendering
When a macOS Seatbelt denial contains a terminal control sequence, seatbelt_diagnostics_darwin.go preserves the complete event as RawLog, and FormatSandboxDetails prints that field verbatim. This display copy escapes Target, Process, and RuleLabel but leaves RawLog unchanged, so pmg sandbox violations explain can still execute attacker-controlled ANSI sequences despite the new sanitization. Escape RawLog (and any other rendered audit-derived fields) in this copy as well.
Useful? React with 👍 / 👎.
| if current, currentErr := p.env.readFile(currentPath); currentErr == nil && isPMGAppArmorProfile(string(current)) { | ||
| return sandbox.ProbeResult{ | ||
| Name: sandbox.ProbeAppArmorUserns, | ||
| Status: sandbox.ProbeStatusOK, | ||
| Summary: "AppArmor profile permits pmg to create user namespaces", |
There was a problem hiding this comment.
Verify userns permission instead of trusting the profile name
When the sysctl is restrictive and any AppArmor profile named pmg is attached, this returns OK even if that profile lacks the required userns rule. /proc/self/attr/apparmor/current reports only the active label and mode, not its permissions, so an old or misconfigured pmg profile still leaves Landlock and Bubblewrap failing while sandbox doctor claims they are usable. Use a capability canary or otherwise verify user-namespace creation before returning OK.
Useful? React with 👍 / 👎.



No description provided.