Skip to content

Commit 93b842b

Browse files
committed
fix(review): harden gate_runs_unconditionally against unclosed blocks (#122)
Code review of the egress-guard hardening: - Clamp the span-advance index so an unclosed `if service ==` block can't slice past the body end and panic (latent: real source is balanced, but the scraper should not crash on a pathological future body). Keep span-to-EOF as the fail-safe direction. - Document that only `if service ==` is detected, not `match service`, so a future match-arm gate isn't silently treated as full. - Add unit cases: gate inside both service blocks (conditional), gate inside and outside (full), and the unclosed-block no-panic regression.
1 parent cbde4fd commit 93b842b

1 file changed

Lines changed: 35 additions & 2 deletions

File tree

  • crates/gitlawb-node/src/api

‎crates/gitlawb-node/src/api/mod.rs‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,9 @@ mod authz_guard {
273273
/// "git-upload-pack"`, leaving `git-receive-pack` ungated) covers a subset of
274274
/// services and must NOT count as a full gate. Other handlers carry no
275275
/// `service ==` discriminator, so for them this matches the plain
276-
/// substring check.
276+
/// substring check. NOTE: only `if service ==` is detected — a
277+
/// `match service { .. }` discriminator is NOT tracked and a gate inside one
278+
/// arm would pass as full; avoid that shape, or extend the span loop below.
277279
fn gate_runs_unconditionally(body: &str, markers: &[&str]) -> bool {
278280
// Brace-matched spans of each `if service == ...` block.
279281
let mut cond_spans: Vec<(usize, usize)> = Vec::new();
@@ -300,7 +302,10 @@ mod authz_guard {
300302
}
301303
}
302304
cond_spans.push((open, end));
303-
search = end + 1;
305+
// On an unclosed block `end` stays at body.len() (the fail-safe
306+
// direction: treat the rest as conditional rather than mask a gate);
307+
// clamp so the next slice can't index past the end and panic.
308+
search = (end + 1).min(body.len());
304309
}
305310
markers.iter().any(|m| {
306311
body.match_indices(m)
@@ -518,10 +523,38 @@ mod authz_guard {
518523
gate_runs_unconditionally(unconditional, &markers),
519524
"an unconditional gate runs for every service"
520525
);
526+
// A gate inside EACH of two service blocks, none outside: still a
527+
// subset (no service clears it unconditionally), so not a full gate.
528+
let both_conditional = "fn f() {\n \
529+
if service == \"git-upload-pack\" { visibility_check(a); }\n \
530+
if service == \"git-receive-pack\" { visibility_check(b); }\n}";
531+
assert!(
532+
!gate_runs_unconditionally(both_conditional, &markers),
533+
"a gate inside every service block is still conditional"
534+
);
535+
// A marker inside one block AND again unconditionally: the
536+
// unconditional occurrence makes it a full gate (exercises the
537+
// match_indices scan past the in-block hit).
538+
let inside_and_outside = "fn f() {\n \
539+
if service == \"git-upload-pack\" { visibility_check(a); }\n \
540+
visibility_check(b);\n}";
541+
assert!(
542+
gate_runs_unconditionally(inside_and_outside, &markers),
543+
"an unconditional occurrence counts even when another is conditional"
544+
);
521545
// No marker at all: not gated.
522546
assert!(!gate_runs_unconditionally(
523547
"fn f() { do_thing(); }",
524548
&markers
525549
));
550+
// An unclosed `if service ==` block (e.g. phantom brace from a string
551+
// literal) must not panic on the slice advance; the span runs to EOF, so
552+
// the in-block marker reads as conditional. Real Rust source is balanced,
553+
// so this only guards the scraper against a future pathological body.
554+
let unclosed = "fn f() { if service == \"x\" { visibility_check(a);";
555+
assert!(
556+
!gate_runs_unconditionally(unclosed, &markers),
557+
"an unclosed service block must not panic and stays conditional"
558+
);
526559
}
527560
}

0 commit comments

Comments
 (0)