harden(danger): fix code-review findings on the Unknown class

Self-review of the fail-closed change surfaced consumers that enumerate
risk classes but were never taught about the new Unknown class, plus two
classifier correctness bugs. Fixes:

1. Sub-agent risk caps ignored Unknown (cmd/odek/subagent.go). The mirror
   riskRank() had no Unknown case (returned 0) and the untrusted + maxRisk
   clamp loops didn't list it. Result: max_risk="unknown" denied even Safe,
   and a capped/untrusted sub-agent never force-denied Unknown when the
   inherited config loosened it. Root cause was the duplicated ranking, so
   export danger.Rank as the single source of truth (rank→Rank) and have
   subagent.go use it; add danger.Unknown to both clamp loops.

2. Trust-class shortcut allowed Unknown (internal/danger/approver.go,
   cmd/odek/wsapprover.go). "Trust class for session" was disabled only for
   Destructive/Blocked, so a single grant could blanket-approve every future
   unrecognised verb — the exact approval-fatigue attack the exclusion
   exists to stop. Exclude Unknown too.

3. dd to a character device misclassified destructive (classifier.go).
   isDestructive's of= branch matched any "/dev/" substring, so the common
   `dd ... of=/dev/null` / `of=/dev/stdout` idioms were denied. Use the
   precise containsBlockDevice matcher (also dedupes the device logic).

4. ANSI-C octal over-read (classifier.go decodeEscape). The loop consumed
   up to 4 octal digits; bash takes at most 3. `$'\1551'` now decodes to
   "m1" (octal 155 + literal 1), matching the shell, not a single byte.

Regression tests added for each; rank→Rank rename propagated through the
danger package and its tests. Full suite green; go vet clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jkyberneees

cmd/odek/subagent.go

@@ -492,8 +492,8 @@ func truncate(s string, n int) string {
 // outside CWD, an MCP server response). We:
 //   - Force NonInteractiveAction to deny (sub-agents have no TTY).
 //   - Clamp the action for Destructive, CodeExecution, Install,
-//     SystemWrite, and NetworkEgress to Deny so the sub-agent cannot
-//     escalate beyond LocalWrite without coming back through the
+//     SystemWrite, NetworkEgress, and Unknown to Deny so the sub-agent
+//     cannot escalate beyond LocalWrite without coming back through the
 //     parent.
 //
 // maxRisk caps the highest risk class the sub-agent will execute.
@@ -522,15 +522,15 @@ func applySubagentTrust(dc *danger.DangerousConfig, trustLevel, maxRisk string)
 			danger.Install,
 			danger.SystemWrite,
 			danger.NetworkEgress,
+			danger.Unknown,
 			danger.Blocked,
 		} {
 			dc.Classes[cls] = danger.Deny
 		}
 	}
 
 	if maxRisk != "" {
-		cap := danger.RiskClass(maxRisk)
-		capRank := riskRank(cap)
+		capRank := danger.Rank(danger.RiskClass(maxRisk))
 		for _, cls := range []danger.RiskClass{
 			danger.Safe,
 			danger.LocalWrite,
@@ -539,35 +539,12 @@ func applySubagentTrust(dc *danger.DangerousConfig, trustLevel, maxRisk string)
 			danger.NetworkEgress,
 			danger.CodeExecution,
 			danger.Install,
+			danger.Unknown,
 			danger.Blocked,
 		} {
-			if riskRank(cls) > capRank {
+			if danger.Rank(cls) > capRank {
 				dc.Classes[cls] = danger.Deny
 			}
 		}
 	}
 }
-
-// riskRank mirrors internal/danger.rank but is duplicated here to keep
-// applySubagentTrust local. Order matches internal/danger/classifier.go.
-func riskRank(cls danger.RiskClass) int {
-	switch cls {
-	case danger.Blocked:
-		return 8
-	case danger.Destructive:
-		return 7
-	case danger.SystemWrite:
-		return 6
-	case danger.CodeExecution:
-		return 5
-	case danger.NetworkEgress:
-		return 4
-	case danger.Install:
-		return 3
-	case danger.LocalWrite:
-		return 2
-	case danger.Safe:
-		return 1
-	}
-	return 0
-}

cmd/odek/subagent_trust_test.go

@@ -35,6 +35,7 @@ func TestApplySubagentTrust_Untrusted_LocksDownEscalationClasses(t *testing.T) {
 		danger.Install,
 		danger.SystemWrite,
 		danger.NetworkEgress,
+		danger.Unknown,
 		danger.Blocked,
 	} {
 		if got := dc.Classes[cls]; got != danger.Deny {
@@ -63,6 +64,7 @@ func TestApplySubagentTrust_MaxRisk_ClampsAbove(t *testing.T) {
 		danger.NetworkEgress,
 		danger.CodeExecution,
 		danger.Install,
+		danger.Unknown,
 		danger.Blocked,
 	} {
 		if got := dc.Classes[cls]; got != danger.Deny {
@@ -76,3 +78,24 @@ func TestApplySubagentTrust_MaxRisk_ClampsAbove(t *testing.T) {
 		}
 	}
 }
+
+// TestApplySubagentTrust_MaxRiskUnknown_KeepsSafeOpen guards the fix for the
+// cap miscomputation: before Unknown was added to riskRank's shared ordering,
+// max_risk="unknown" computed rank 0 and force-denied even Safe. It must now
+// leave Safe/LocalWrite open and deny only the classes above Unknown.
+func TestApplySubagentTrust_MaxRiskUnknown_KeepsSafeOpen(t *testing.T) {
+	dc := danger.DangerousConfig{}
+	applySubagentTrust(&dc, "", "unknown")
+
+	for _, cls := range []danger.RiskClass{danger.Safe, danger.LocalWrite} {
+		if got, ok := dc.Classes[cls]; ok && got == danger.Deny {
+			t.Errorf("Class %s must stay open with max_risk=unknown, got %q", cls, got)
+		}
+	}
+	// Only classes ranked above Unknown (Destructive, Blocked) are denied.
+	for _, cls := range []danger.RiskClass{danger.Destructive, danger.Blocked} {
+		if got := dc.Classes[cls]; got != danger.Deny {
+			t.Errorf("Class %s = %q, want deny with max_risk=unknown", cls, got)
+		}
+	}
+}

cmd/odek/wsapprover.go

@@ -36,11 +36,13 @@ type approvalRequest struct {
 	FrictionApprovals int    `json:"friction_approvals,omitempty"`
 }
 
-// allowTrustForClass mirrors the TTYApprover policy: destructive and
-// blocked must never be class-trusted, so an attacker cannot social-
-// engineer a single broad approval into long-term carte blanche.
+// allowTrustForClass mirrors the TTYApprover policy: destructive, blocked,
+// and unknown must never be class-trusted, so an attacker cannot social-
+// engineer a single broad approval into long-term carte blanche. Unknown is
+// the fail-closed catch-all for unrecognised verbs; class-trusting it would
+// blanket-approve every future obfuscated/novel command.
 func allowTrustForClass(cls danger.RiskClass) bool {
-	return cls != danger.Destructive && cls != danger.Blocked
+	return cls != danger.Destructive && cls != danger.Blocked && cls != danger.Unknown
 }
 
 // approvalResponse is received from the browser when the user responds.

cmd/odek/wsapprover_test.go

@@ -348,6 +348,7 @@ func TestWSApprover_AllowTrustFlag_PerClass(t *testing.T) {
 		{danger.Install, true},
 		{danger.Destructive, false},
 		{danger.Blocked, false},
+		{danger.Unknown, false},
 	}
 	for _, tc := range cases {
 		t.Run(string(tc.cls), func(t *testing.T) {
@@ -433,3 +434,18 @@ func TestWSApprover_TrustResponse_CoercedToApprove_ForBlocked(t *testing.T) {
 		t.Error("blocked class was cached as trusted — class trust must be impossible")
 	}
 }
+
+// TestWSApprover_TrustResponse_CoercedToApprove_ForUnknown verifies the
+// fail-closed Unknown class cannot be class-trusted: a forged "trust" is
+// treated as a single approve and never cached, so unrecognised verbs can't
+// be blanket-approved by one social-engineered grant.
+func TestWSApprover_TrustResponse_CoercedToApprove_ForUnknown(t *testing.T) {
+	a := newWSApprover(nil)
+	_, err := promptAndCaptureRequest(t, a, danger.Unknown, "trust")
+	if err != nil {
+		t.Errorf("expected nil error (coerced to approve), got: %v", err)
+	}
+	if a.approveAll[danger.Unknown] {
+		t.Error("unknown class was cached as trusted — class trust must be impossible")
+	}
+}

internal/danger/approver.go

@@ -144,12 +144,13 @@ func (a *TTYApprover) prompt(cls RiskClass, cmd, description string) error {
 	}
 	defer tty.Close()
 
-	// Trust-class shortcut is disabled for the two highest-impact
-	// classes. Destructive and Blocked operations always require a
-	// per-call approval to defeat approval-fatigue attacks where the
-	// model batches a benign destructive-class trust grant with a
-	// destructive payload.
-	allowTrust := cls != Destructive && cls != Blocked
+	// Trust-class shortcut is disabled for the highest-impact classes.
+	// Destructive and Blocked always require per-call approval to defeat
+	// approval-fatigue attacks where the model batches a benign trust grant
+	// with a dangerous payload. Unknown is included because it is the
+	// fail-closed catch-all for unrecognised verbs — class-trusting it would
+	// blanket-approve every future obfuscated/novel command.
+	allowTrust := cls != Destructive && cls != Blocked && cls != Unknown
 
 	// Approval-fatigue mitigation: if the user has already approved
 	// this class FrictionThreshold times in FrictionWindow, the next

internal/danger/classifier.go

@@ -708,7 +708,7 @@ func Classify(cmd string) RiskClass {
 		// Substitutions are themselves commands the shell will run.
 		// Re-enter Classify (not classifyOne) so nested substitutions
 		// inside them also normalise.
-		if r := Classify(s); rank(r) > rank(worst) {
+		if r := Classify(s); Rank(r) > Rank(worst) {
 			worst = r
 		}
 	}
@@ -728,7 +728,7 @@ func classifyOne(cmd string) RiskClass {
 	worst := Safe
 	for _, seg := range segments {
 		cls := classifyPipeline(seg)
-		if rank(cls) > rank(worst) {
+		if Rank(cls) > Rank(worst) {
 			worst = cls
 		}
 	}
@@ -876,13 +876,17 @@ func decodeEscape(s string, b *strings.Builder) int {
 			}
 		}
 	default:
-		if s[1] >= '0' && s[1] <= '7' { // \NNN octal (1–3 digits)
+		if s[1] >= '0' && s[1] <= '7' { // \NNN octal (1–3 digits, like bash)
+			// end starts after the backslash+first digit; cap at end<4 so at
+			// most 3 octal digits (s[1:4]) are consumed. A wider bound would
+			// swallow a following literal octal digit and diverge from the
+			// shell (bash: $'\1551' → "m1", not one byte).
 			end := 2
-			for end < len(s) && end < 5 && s[end] >= '0' && s[end] <= '7' {
+			for end < len(s) && end < 4 && s[end] >= '0' && s[end] <= '7' {
 				end++
 			}
 			if v, err := strconv.ParseUint(s[1:end], 8, 16); err == nil {
-				b.WriteByte(byte(v))
+				b.WriteByte(byte(v)) // bash takes octal escapes mod 256
 				return end
 			}
 		}
@@ -1284,7 +1288,7 @@ func commandName(tok string) string {
 
 // worstOf returns whichever class ranks higher (more severe).
 func worstOf(a, b RiskClass) RiskClass {
-	if rank(b) > rank(a) {
+	if Rank(b) > Rank(a) {
 		return b
 	}
 	return a
@@ -1524,9 +1528,12 @@ func isDestructive(first string, tokens []string) bool {
 		return len(tokens) >= 1
 	}
 
-	// dd with of=/dev/sd* or of=/dev/nvme*
+	// dd writing to a raw block device (of=/dev/sda etc.) is destructive.
+	// Match only real block devices via containsBlockDevice/isBlockDevice —
+	// NOT any "/dev/" substring, so benign discards like of=/dev/null and
+	// of=/dev/stdout are not misclassified.
 	for _, tok := range tokens {
-		if strings.HasPrefix(tok, "of=") && strings.Contains(tok, "/dev/") {
+		if strings.HasPrefix(tok, "of=") && containsBlockDevice(tok) {
 			return true
 		}
 		if tok == "of=" && len(tokens) > 1 {
@@ -1754,8 +1761,11 @@ func isSystemPath(path string) bool {
 
 // ── Ranking ────────────────────────────────────────────────────────────
 
-// rank returns the severity order for priority comparison.
-func rank(cls RiskClass) int {
+// Rank returns the severity order for priority comparison. Exported so
+// consumers that enforce risk caps (e.g. the sub-agent maxRisk clamp) share
+// this single ordering instead of mirroring it — a mirror silently drifts
+// when a class is added, as happened with Unknown.
+func Rank(cls RiskClass) int {
 	switch cls {
 	case Blocked:
 		return 9

internal/danger/classifier_bypass_test.go

@@ -76,7 +76,7 @@ func TestBypass_KnownEvasions(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			got := Classify(tc.cmd)
-			if rank(got) < rank(tc.minWant) {
+			if Rank(got) < Rank(tc.minWant) {
 				t.Errorf("Classify(%q) = %v, want at least %v\nwhy: %s",
 					tc.cmd, got, tc.minWant, tc.why)
 			}
@@ -99,7 +99,7 @@ func TestBypass_KnownDetections(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			got := Classify(tc.cmd)
-			if rank(got) < rank(Destructive) {
+			if Rank(got) < Rank(Destructive) {
 				t.Errorf("Classify(%q) = %v, want >= destructive — regression in existing detection", tc.cmd, got)
 			}
 		})

internal/danger/classifier_test.go

@@ -831,9 +831,9 @@ func TestRank(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := rank(tt.cls)
+			got := Rank(tt.cls)
 			if got != tt.want {
-				t.Errorf("rank(%s) = %d, want %d", tt.cls, got, tt.want)
+				t.Errorf("Rank(%s) = %d, want %d", tt.cls, got, tt.want)
 			}
 		})
 	}

internal/danger/hardening_test.go

@@ -88,7 +88,7 @@ func TestHardening_ReverseShellDevTCP(t *testing.T) {
 		"exec 3<>/dev/udp/10.0.0.1/53",
 	}
 	for _, cmd := range cases {
-		if got := Classify(cmd); rank(got) < rank(NetworkEgress) {
+		if got := Classify(cmd); Rank(got) < Rank(NetworkEgress) {
 			t.Errorf("Classify(%q) = %s, want >= network_egress", cmd, got)
 		}
 	}
@@ -133,7 +133,7 @@ func TestHardening_SensitivePathReads(t *testing.T) {
 		"grep secret ~/.git-credentials",
 	}
 	for _, cmd := range cases {
-		if got := Classify(cmd); rank(got) < rank(SystemWrite) {
+		if got := Classify(cmd); Rank(got) < Rank(SystemWrite) {
 			t.Errorf("Classify(%q) = %s, want >= system_write", cmd, got)
 		}
 	}
@@ -239,6 +239,42 @@ func TestHardening_SafeAllowlistStillSafe(t *testing.T) {
 	}
 }
 
+func TestHardening_DdToCharDeviceNotDestructive(t *testing.T) {
+	// dd writing to a character pseudo-device (discard/echo) is benign — only
+	// raw block devices are destructive. Loose "/dev/" matching used to deny
+	// these common idioms.
+	benign := []string{
+		"dd if=/dev/zero of=/dev/null bs=1M count=100",
+		"dd if=/dev/urandom of=/dev/stdout count=1",
+	}
+	for _, cmd := range benign {
+		if got := Classify(cmd); got == Destructive || got == Blocked {
+			t.Errorf("Classify(%q) = %s, want non-destructive", cmd, got)
+		}
+	}
+	// Real block-device writes stay destructive/blocked.
+	for _, cmd := range []string{"dd if=/dev/zero of=/dev/sda", "dd if=/dev/zero of=/dev/nvme0n1 bs=4M"} {
+		if got := Classify(cmd); Rank(got) < Rank(Destructive) {
+			t.Errorf("Classify(%q) = %s, want >= destructive", cmd, got)
+		}
+	}
+}
+
+func TestHardening_ANSICOctalDigitCap(t *testing.T) {
+	// $'\NNN' consumes at most 3 octal digits, like bash: $'\1551' is octal
+	// 155 ('m') followed by a literal '1' → "m1", not a single over-read byte.
+	cases := map[string]string{
+		`$'\155'`:     "m",
+		`$'\1551'`:    "m1",
+		`$'\162\155'`: "rm",
+	}
+	for in, want := range cases {
+		if got := decodeANSIC(in); got != want {
+			t.Errorf("decodeANSIC(%s) = %q, want %q", in, got, want)
+		}
+	}
+}
+
 // TestHardening_NoRegressionOnBenign guards against over-classification of
 // ordinary developer commands that must remain low-risk.
 func TestHardening_NoRegressionOnBenign(t *testing.T) {