Skip to content

Commit fae4909

Browse files
mikeraclaude
andcommitted
fix(security): boundary-aware Capability.resourceCovers + fail-closed empty with (#585)
Capability.resourceCovers matched a granted resource against a requested one with a raw string startsWith and no path-segment boundary, so a grant on `w/notes` also covered siblings like `w/notesSECRET` / `w/report-confidential` — a UCAN attenuation / delegation escape (cross-tenant over-read/over-write where delegation is used). Separately, a null/empty `with` was treated as "covers every resource" (fail-open), turning a malformed or truncated capability into a superuser grant. Fix: - resourceCovers now requires a path-segment boundary on the prefix branch (mirroring the already-correct abilityCovers): the prefix matches only when the grant ends with '/' or the next char in the request is '/'. Exact match and trailing-slash-parent are preserved; genuine children (`w/notes/x`) still match. - Both resourceCovers and covers now fail closed on null/empty grant or request `with`. An absent resource pointer is never a wildcard. Tests (CapabilityTest): adversarial sibling-escape and empty/null-`with` superuser cases, plus genuine-child/exact regressions. The five existing tests that asserted the fail-open behaviour are flipped to expect fail-closed. Confirmed all seven adversarial assertions fail against the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b58503b commit fae4909

2 files changed

Lines changed: 66 additions & 28 deletions

File tree

convex-core/src/main/java/convex/auth/ucan/Capability.java

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,10 @@ public static boolean covers(AString grantWith, AString grantCan,
9191
AString requestWith, AString requestCan) {
9292
if (grantCan == null || requestCan == null) return false;
9393

94-
// Resource attenuation
95-
if (grantWith != null && requestWith != null) {
96-
if (!resourceCovers(grantWith, requestWith)) return false;
97-
}
98-
// null grantWith = any resource (wildcard)
94+
// Resource attenuation. Fail-closed: a grant with no resource pointer (null/empty
95+
// `with`) covers nothing — absence is NOT a resource wildcard. Resource wildcards
96+
// must be explicit, never implied by a missing/truncated capability.
97+
if (!resourceCovers(grantWith, requestWith)) return false;
9998

10099
// Ability attenuation
101100
return abilityCovers(grantCan, requestCan);
@@ -104,33 +103,40 @@ public static boolean covers(AString grantWith, AString grantCan,
104103
/**
105104
* Checks whether a granted resource path covers a requested resource path.
106105
*
107-
* <p>Uses path prefix matching with boundary awareness:</p>
106+
* <p>Uses path prefix matching with path-segment boundary awareness:</p>
108107
* <ul>
109108
* <li>Exact match: {@code "w/decisions"} covers {@code "w/decisions"}</li>
110-
* <li>Prefix: {@code "w/decisions"} covers {@code "w/decisions/INV-123"}</li>
109+
* <li>Prefix at a segment boundary: {@code "w/decisions"} covers {@code "w/decisions/INV-123"},
110+
* but NOT the sibling {@code "w/decisions-secret"}</li>
111111
* <li>Trailing slash: {@code "w/decisions/"} covers {@code "w/decisions"} and children</li>
112-
* <li>Empty grant covers any resource</li>
113112
* </ul>
113+
*
114+
* <p><b>Fail-closed:</b> a null or empty {@code grant} or {@code request} covers nothing.
115+
* An absent/empty resource pointer is never a wildcard.</p>
114116
*/
115117
public static boolean resourceCovers(AString grant, AString request) {
116-
if (grant == null) return true;
117-
if (request == null) return true;
118+
// Fail-closed: a missing or empty resource pointer grants nothing (not a wildcard).
119+
if (grant == null || request == null) return false;
118120

119121
long gLen = grant.count();
120122
long rLen = request.count();
121-
122-
// Empty grant covers everything
123-
if (gLen == 0) return true;
123+
if (gLen == 0 || rLen == 0) return false;
124124

125125
// Exact match (interned strings may be identical objects)
126126
if (grant.equals(request)) return true;
127127

128-
// Check if request starts with grant
129-
if (rLen > gLen && request.startsWith(grant)) return true;
128+
// Trailing-slash grant: "w/records/" also covers the bare parent "w/records"
129+
if (grant.charAt(gLen - 1) == '/' && rLen == gLen - 1
130+
&& request.equals(grant.slice(0, gLen - 1))) {
131+
return true;
132+
}
130133

131-
// Trailing slash: "w/records/" covers "w/records"
132-
if (gLen > 0 && grant.charAt(gLen - 1) == '/' && rLen == gLen - 1) {
133-
if (request.equals(grant.slice(0, gLen - 1))) return true;
134+
// Prefix, but only at a path-segment boundary, so "w/notes" covers "w/notes/x" but NOT
135+
// the sibling "w/notesSECRET". The boundary holds when the grant already ends with '/',
136+
// or the next character in the request is '/'.
137+
if (rLen > gLen && request.startsWith(grant)
138+
&& (grant.charAt(gLen - 1) == '/' || request.charAt(gLen) == '/')) {
139+
return true;
134140
}
135141

136142
return false;

convex-core/src/test/java/convex/auth/ucan/CapabilityTest.java

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,41 @@ public void testResourceNotCovered() {
109109
}
110110

111111
@Test
112-
public void testResourceNullGrantCoversAll() {
113-
assertTrue(Capability.resourceCovers(null, Strings.create("w/anything")));
112+
public void testResourceNullGrantFailsClosed() {
113+
// A grant with no resource pointer covers nothing — absence is not a wildcard (#585).
114+
assertFalse(Capability.resourceCovers(null, Strings.create("w/anything")));
114115
}
115116

116117
@Test
117-
public void testResourceEmptyGrantCoversAll() {
118-
assertTrue(Capability.resourceCovers(Strings.create(""), Strings.create("w/anything")));
118+
public void testResourceEmptyGrantFailsClosed() {
119+
assertFalse(Capability.resourceCovers(Strings.create(""), Strings.create("w/anything")));
119120
}
120121

121122
@Test
122-
public void testResourceNullRequestAllowed() {
123-
assertTrue(Capability.resourceCovers(Strings.create("w/data"), null));
123+
public void testResourceNullRequestFailsClosed() {
124+
assertFalse(Capability.resourceCovers(Strings.create("w/data"), null));
125+
}
126+
127+
@Test
128+
public void testResourceEmptyRequestFailsClosed() {
129+
assertFalse(Capability.resourceCovers(Strings.create("w/data"), Strings.create("")));
130+
}
131+
132+
// ----- Adversarial: path-segment boundary (attenuation escape, #585) -----
133+
134+
@Test
135+
public void testResourceSiblingNotCovered() {
136+
// A grant on "w/notes" must NOT cover siblings that merely share the string prefix.
137+
assertFalse(Capability.resourceCovers(Strings.create("w/notes"), Strings.create("w/notesSECRET")));
138+
assertFalse(Capability.resourceCovers(Strings.create("w/report"), Strings.create("w/report-confidential")));
139+
assertFalse(Capability.resourceCovers(Strings.create("w/report"), Strings.create("w/reports")));
140+
}
141+
142+
@Test
143+
public void testResourceChildStillCoveredAtBoundary() {
144+
// But it MUST still cover genuine path children and the exact resource.
145+
assertTrue(Capability.resourceCovers(Strings.create("w/report"), Strings.create("w/report/2024")));
146+
assertTrue(Capability.resourceCovers(Strings.create("w/report"), Strings.create("w/report")));
124147
}
125148

126149
// ========== Full covers (AString) ==========
@@ -177,13 +200,22 @@ public void testCoversString() {
177200
}
178201

179202
@Test
180-
public void testCoversStringWildcard() {
181-
assertTrue(Capability.covers("", "*", "w/anything", "crud/write"));
203+
public void testCoversEmptyWithFailsClosed() {
204+
// An empty grant resource must not become an all-resources grant, even with TOP ability (#585).
205+
assertFalse(Capability.covers("", "*", "w/anything", "crud/write"));
206+
}
207+
208+
@Test
209+
public void testCoversNullWithFailsClosed() {
210+
assertFalse(Capability.covers((String) null, "*", "w/anything", "crud/read"));
182211
}
183212

184213
@Test
185-
public void testCoversStringNull() {
186-
assertTrue(Capability.covers((String) null, "*", "w/anything", "crud/read"));
214+
public void testCoversSiblingResourceEscapeBlocked() {
215+
// crud/read on "w/report" must not reach the sibling "w/report-confidential" (#585).
216+
assertFalse(Capability.covers("w/report", "crud/read", "w/report-confidential", "crud/read"));
217+
// but still covers the genuine child
218+
assertTrue(Capability.covers("w/report", "crud/read", "w/report/q3", "crud/read"));
187219
}
188220

189221
// ========== DID URL resources ==========

0 commit comments

Comments
 (0)