Repository navigation
Learning proposal: (ba730dfb) #3890
Description
Source PR: merge commit ba730dfb0dbda69aa651f8145466d09ca9a36916 (2 substantive review rounds)
What the review rounds surfaced. The reviewer's framing of why the fix was right is the durable part: "every prior round relocated the trust decision to a different field of the same untrusted artifact. There is no fifth field." A lockfile-integrity gate carried an exemption for locally-sourced plugin entries, and successive hardening attempts kept moving which field of the attacker-supplied entry was consulted to decide whether the exemption applied. Each round produced a narrower forgeable field rather than no forgeable field. Deleting the exemption terminated the series because it removed the decision, not just the current spelling of it.
The reusable learning. When a security fix keeps recurring with the same shape — round after round tightening which field of an untrusted input authorizes a bypass — the recurrence itself is the signal that the bypass is the bug. The terminating move is eliminating the trust decision, not refining its input. A useful corroborating check appears here too: the reviewer established that the code had shipped without the exemption for months and stayed green, meaning the exemption was speculative hardening for a shape that never occurs in practice. Deleting it was a return to a configuration with real green evidence behind it, plus an explicit rejection where there had been silence. Before defending an exemption, check whether it has ever fired.
The secondary design lesson. The review credited a separate predicate for earning its existence: keeping "is this path-shaped" (mirroring the upstream classifier) apart from "is this an acceptable path" is precisely the discipline that was missing when one predicate answered both questions. Conflated predicates are how the trust decision got smuggled into a classifier in the first place. The containment check also uses resolution-based comparison with a resolvedRoot + separator guard rather than a bare startsWith, which would treat sibling directories sharing a name prefix as contained.
Proposed learning to author: a docs/solutions/ entry on recognizing the "relocated trust decision" anti-pattern — repeated hardening rounds on different fields of the same untrusted artifact mean the exemption should be deleted — plus the two supporting practices: check whether an exemption has ever fired in production before defending it, and keep shape-classification predicates separate from acceptability predicates.
Activity