Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions cmd/sandbox/violations_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,21 @@ func renderViolationsListTable(out io.Writer, entries []pmgsandbox.ViolationCach

sandboxName := ""
profile := ""
var primary *pmgsandbox.Violation
if e.Record.Report != nil {
sandboxName = string(e.Record.Report.SandboxName)
profile = e.Record.Report.PolicyName
primary = ui.SandboxViolationForTerminal(pmgsandbox.BuildExplanation(e.Record.Report).Primary)
}

kind := dash
target := dash
if e.Record.Report != nil {
primary := pmgsandbox.BuildExplanation(e.Record.Report).Primary
if primary != nil {
if primary.Kind != "" {
kind = string(primary.Kind)
}
if primary.Target != "" {
target = truncate(primary.Target, 60)
}
if primary != nil {
if primary.Kind != "" {
kind = string(primary.Kind)
}
if primary.Target != "" {
target = truncate(primary.Target, 60)
}
}

Expand Down
12 changes: 12 additions & 0 deletions cmd/sandbox/violations_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ func TestViolationsList_TwoEntriesNewestFirst(t *testing.T) {
assert.Less(t, newerIdx, olderIdx, "newest entry should appear first")
}

func TestViolationsListEscapesAuditControls(t *testing.T) {
factory, cache := newTestCacheFactory(t)

_, err := cache.Write(sampleViolationsReport("/tmp/bad\npath\x1b[2J"))
require.NoError(t, err)

stdout, _, err := runList(t, factory)
require.NoError(t, err)
assert.Contains(t, stdout, `bad\npath\x1b[2J`)
assert.NotContains(t, stdout, "\x1b")
}

func TestViolationsList_LimitOne(t *testing.T) {
factory, cache := newTestCacheFactory(t)

Expand Down
37 changes: 29 additions & 8 deletions internal/ui/sandbox_violation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ui
import (
"fmt"
"io"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -97,6 +98,7 @@ func RenderSandboxViolation(out io.Writer, rec *pmgsandbox.ViolationCacheRecord)
}

exp := pmgsandbox.BuildExplanation(rec.Report)
primary := SandboxViolationForTerminal(exp.Primary)

recordedAt := ""
if !rec.RecordedAt.IsZero() {
Expand All @@ -121,7 +123,7 @@ func RenderSandboxViolation(out io.Writer, rec *pmgsandbox.ViolationCacheRecord)
return err
}

hint := FormatSandboxHint(exp.Primary, exp.Override)
hint := FormatSandboxHint(primary, exp.Override)
if hint != "" {
if _, err := fmt.Fprintln(out, hint); err != nil {
return err
Expand All @@ -131,7 +133,7 @@ func RenderSandboxViolation(out io.Writer, rec *pmgsandbox.ViolationCacheRecord)
}
}

details := FormatSandboxDetails(rec.Report, exp.Primary)
details := FormatSandboxDetails(rec.Report, primary)
if details != "" {
if _, err := fmt.Fprintln(out, Colors.Bold("Details:")); err != nil {
return err
Expand Down Expand Up @@ -168,21 +170,21 @@ func RenderSandboxViolation(out io.Writer, rec *pmgsandbox.ViolationCacheRecord)
}
}

if exp.Primary != nil {
if primary != nil {
if _, err := fmt.Fprintln(out, Colors.Bold("Primary violation:")); err != nil {
return err
}
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Kind:"), string(exp.Primary.Kind)); err != nil {
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Kind:"), string(primary.Kind)); err != nil {
return err
}
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Target:"), exp.Primary.Target); err != nil {
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Target:"), primary.Target); err != nil {
return err
}
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Rule:"), exp.Primary.RuleLabel); err != nil {
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Rule:"), primary.RuleLabel); err != nil {
return err
}
if exp.Primary.Process != "" {
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Process:"), exp.Primary.Process); err != nil {
if primary.Process != "" {
if _, err := fmt.Fprintf(out, " %s %s\n", Colors.Dim("Process:"), primary.Process); err != nil {
return err
}
}
Expand All @@ -191,6 +193,25 @@ func RenderSandboxViolation(out io.Writer, rec *pmgsandbox.ViolationCacheRecord)
return nil
}

// SandboxViolationForTerminal returns a display-only copy with audit-controlled
// strings escaped. Structured JSON output should continue using the original.
func SandboxViolationForTerminal(violation *pmgsandbox.Violation) *pmgsandbox.Violation {
if violation == nil {
return nil
}

safe := *violation
safe.Target = quoteTerminalText(safe.Target)
safe.Process = quoteTerminalText(safe.Process)
safe.RuleLabel = quoteTerminalText(safe.RuleLabel)
Comment on lines +203 to +206

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

return &safe
}

func quoteTerminalText(value string) string {
quoted := strconv.QuoteToGraphic(value)
return quoted[1 : len(quoted)-1]
}

// shellQuote wraps value in single quotes, escaping any embedded single
// quotes. Used so suggested override flags can be copy-pasted into a POSIX
// shell verbatim regardless of spaces or quotes in the target.
Expand Down
34 changes: 34 additions & 0 deletions internal/ui/sandbox_violation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ func TestFormatSandboxHintEmpty(t *testing.T) {
assert.Equal(t, "Reason: sandbox denied an operation", FormatSandboxHint(nil, nil))
}

func TestSandboxViolationForTerminalEscapesControlCharacters(t *testing.T) {
violation := &pmgsandbox.Violation{
Target: "bad\npath\x1b[2J\tend\x7f",
}

safe := SandboxViolationForTerminal(violation)
assert.Equal(t, `bad\npath\x1b[2J\tend\x7f`, safe.Target)
assert.Equal(t, "bad\npath\x1b[2J\tend\x7f", violation.Target)
}

func TestFormatSandboxHintIncludesOverride(t *testing.T) {
primary := &pmgsandbox.Violation{
Kind: pmgsandbox.ViolationKindFSRead,
Expand All @@ -60,6 +70,30 @@ func TestFormatSandboxHintIncludesOverride(t *testing.T) {
assert.Contains(t, hint, "Override: --sandbox-allow read='./.env'")
}

func TestRenderSandboxViolationEscapesAuditControls(t *testing.T) {
rec := &pmgsandbox.ViolationCacheRecord{
SchemaVersion: pmgsandbox.ViolationCacheSchemaVersion,
Report: &pmgsandbox.ViolationReport{
SandboxName: pmgsandbox.DriverLandlock,
PolicyName: "test",
Violations: []pmgsandbox.Violation{{
Kind: pmgsandbox.ViolationKindFSRead,
Target: "/tmp/bad\npath\x1b[2J",
RuleTarget: "/tmp/rule",
RuleLabel: "read access denied: /tmp/bad\npath\x1b[2J",
Process: "bad\nprocess",
}},
},
}

var buf bytes.Buffer
require.NoError(t, RenderSandboxViolation(&buf, rec))
out := buf.String()
assert.Contains(t, out, `bad\npath\x1b[2J`)
assert.Contains(t, out, `bad\nprocess`)
assert.NotContains(t, out, "\x1b")
}

func TestFormatSandboxDetailsIncludesMatchedRule(t *testing.T) {
report := &pmgsandbox.ViolationReport{
SandboxName: "seatbelt",
Expand Down
7 changes: 4 additions & 3 deletions sandbox/all_overrides_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ func TestBuildAllOverridesMapsAllSafeFSAndExec(t *testing.T) {
Violations: []Violation{
{Kind: ViolationKindFSWrite, Target: "/repo/.astro"},
{Kind: ViolationKindExec, Target: "/usr/bin/sh"},
{Kind: ViolationKindFSWrite, Target: "/repo/.astro"}, // duplicate collapsed
{Kind: ViolationKindGenericDeny, Target: "/something/else"}, // unsupported kind dropped
{Kind: ViolationKindFSRead, Target: "**/*.env"}, // unsafe target dropped
{Kind: ViolationKindFSWrite, Target: "/repo/.astro"}, // duplicate collapsed
{Kind: ViolationKindGenericDeny, Target: "/something/else"}, // unsupported kind dropped
{Kind: ViolationKindFSRead, Target: "**/*.env"}, // unsafe target dropped
{Kind: ViolationKindFSRead, Target: "/repo/.env", RuleTarget: "**/.env"},
},
}
got := BuildAllOverrides(report)
Expand Down
54 changes: 32 additions & 22 deletions sandbox/platform/landlock_diagnostics_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,23 @@ func extractLandlockViolations(events []capturedAuditEvent) []sandbox.Violation
continue
}

key := landlockDenyKey(e.auditEvent)
if seen[key] {
continue
for _, kind := range landlockViolationKinds(e.auditEvent) {
key := string(kind) + "\x00" + e.Path
if seen[key] {
continue
}
seen[key] = true

violations = append(violations, sandbox.Violation{
Kind: kind,
RawKind: e.Syscall,
Target: e.Path,
RuleTarget: e.RulePath,
Process: e.Comm,
RawLog: e.raw,
RuleLabel: summarizeLandlockViolation(kind, e.Path),
})
}
seen[key] = true

kind := landlockViolationKind(e.auditEvent)

violations = append(violations, sandbox.Violation{
Kind: kind,
RawKind: e.Syscall,
Target: e.Path,
RuleTarget: e.RulePath,
Process: e.Comm,
RawLog: e.raw,
RuleLabel: summarizeLandlockViolation(kind, e.Path),
})
}

return violations
Expand All @@ -159,20 +159,30 @@ func extractLandlockViolations(events []capturedAuditEvent) []sandbox.Violation
// (captureAuditEvents) and extract-time dedupe (extractLandlockViolations)
// must agree on this identity, so both use this function.
func landlockDenyKey(e auditEvent) string {
return string(landlockViolationKind(e)) + "\x00" + e.Path
key := ""
for _, kind := range landlockViolationKinds(e) {
key += string(kind) + "\x00"
}
return key + e.Path
}

func landlockViolationKind(e auditEvent) sandbox.ViolationKind {
func landlockViolationKinds(e auditEvent) []sandbox.ViolationKind {
switch e.Syscall {
case "execve", "execveat":
return sandbox.ViolationKindExec
return []sandbox.ViolationKind{sandbox.ViolationKindExec}
case "openat", "openat2":
if e.Access == "read" {
return sandbox.ViolationKindFSRead
return []sandbox.ViolationKind{sandbox.ViolationKindFSRead}
}
if e.Access == "read_write" {
return []sandbox.ViolationKind{
sandbox.ViolationKindFSRead,
sandbox.ViolationKindFSWrite,
}
}
return sandbox.ViolationKindFSWrite
return []sandbox.ViolationKind{sandbox.ViolationKindFSWrite}
default:
return sandbox.ViolationKindGenericDeny
return []sandbox.ViolationKind{sandbox.ViolationKindGenericDeny}
}
}

Expand Down
24 changes: 23 additions & 1 deletion sandbox/platform/landlock_diagnostics_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,28 @@ func TestExtractLandlockViolations(t *testing.T) {
},
},
},
{
name: "read write denial maps to both permissions",
events: []capturedAuditEvent{denyEvent("openat", "/home/dev/.npmrc", "read_write", "node")},
want: []sandbox.Violation{
{
Kind: sandbox.ViolationKindFSRead,
RawKind: "openat",
Target: "/home/dev/.npmrc",
Process: "node",
RawLog: `{"type":"seccomp_deny"}`,
RuleLabel: "read access denied: /home/dev/.npmrc",
},
{
Kind: sandbox.ViolationKindFSWrite,
RawKind: "openat",
Target: "/home/dev/.npmrc",
Process: "node",
RawLog: `{"type":"seccomp_deny"}`,
RuleLabel: "write access denied: /home/dev/.npmrc",
},
},
},
}

for _, tc := range tests {
Expand Down Expand Up @@ -256,7 +278,7 @@ func TestDenyAccessLabelReportsFiredRule(t *testing.T) {

assert.Equal(t, "write", denyAccessLabel(denyWrite, unix.O_RDWR))
assert.Equal(t, "read", denyAccessLabel(denyBoth, unix.O_RDONLY))
assert.Equal(t, "write", denyAccessLabel(denyBoth, unix.O_RDWR))
assert.Equal(t, "read_write", denyAccessLabel(denyBoth, unix.O_RDWR))
}

func TestBestEffortViolationNilOnSuccess(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion sandbox/platform/landlock_helper_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestReadLandlockPolicyFromFile(t *testing.T) {
{Path: "/tmp", Access: 0xFF},
},
DenyPaths: []denyPathEntry{
{Path: "/home/user/.ssh/", Mode: denyBoth},
{Path: "/home/user/.ssh/", RulePath: "$HOME/.ssh/**", Mode: denyBoth},
},
DenyExecPaths: []string{"/usr/bin/curl"},
AllowPTY: true,
Expand Down Expand Up @@ -54,6 +54,7 @@ func TestReadLandlockPolicyFromFile(t *testing.T) {
assert.Equal(t, uint64(0x0C), got.FilesystemRules[0].Access)
assert.Len(t, got.DenyPaths, 1)
assert.Equal(t, "/home/user/.ssh/", got.DenyPaths[0].Path)
assert.Equal(t, "$HOME/.ssh/**", got.DenyPaths[0].RulePath)
assert.Equal(t, denyBoth, got.DenyPaths[0].Mode)
assert.Equal(t, []string{"/usr/bin/curl"}, got.DenyExecPaths)
assert.True(t, got.AllowPTY)
Expand Down
Loading
Loading