Skip to content

Commit db2572f

Browse files
committed
docs(snapstart): stop calling the guard "fail-closed" without qualification
The guard's summary line and a test-section banner called it "strict, fail-closed" while the detailed doc a dozen lines below correctly explains that the REQUEST side forwards an undecidable path rather than 403-ing it. Only the configured side fails closed, and the review bot has now twice read the summary, concluded the request side returns 403 on doubt, and re-raised it as a contradiction -- quoting doc text that was already replaced. The risk is not the confusion itself but the obvious "fix" it invites: turning the request-side pass-through into a 403 would reinstate the /reports/100%25 false-403 regression that pass-through exists to prevent. Both labels now state which half does which, the summary points at the reasoning rather than summarizing it away, and the term is gone from the codebase entirely (the remaining historical use in a test comment is reworded to "undecidable branch"). Comment-only; no behavior change.
1 parent 85ad8cb commit db2572f

1 file changed

Lines changed: 12 additions & 4 deletions

File tree

‎src/lib.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,15 @@ fn percent_decode_once(input: &str) -> Option<String> {
580580
String::from_utf8(out).ok()
581581
}
582582

583-
/// Canonicalize a path into a list of lowercased segments for the strict,
584-
/// fail-closed SnapStart hook guard.
583+
/// Canonicalize a path into a list of lowercased segments for the strict SnapStart
584+
/// hook guard.
585+
///
586+
/// "Strict" means the blocked equivalence class is deliberately wide, NOT that every
587+
/// input is blocked on doubt. The two halves differ and the difference is
588+
/// load-bearing: the **configured** side fails closed (an unguardable hook path is
589+
/// rejected at startup by [`hook_target`]), while the **request** side fails open (an
590+
/// undecidable request path is forwarded, not 403'd). See the `None` discussion below
591+
/// for why forwarding is safe — do not "tighten" it into a 403 without reading it.
585592
///
586593
/// The guard must block *every* spelling that the downstream app router would
587594
/// resolve to the configured hook route, so this over-approximates: it
@@ -2665,7 +2672,8 @@ mod tests {
26652672
}
26662673

26672674
// ---------------------------------------------------------------------
2668-
// Strict fail-closed SnapStart hook-path guard
2675+
// SnapStart hook-path guard: configured side fails closed (rejected at
2676+
// startup), request side fails open (undecidable paths are forwarded).
26692677
// ---------------------------------------------------------------------
26702678

26712679
/// Build an ALB request for an arbitrary method + raw path.
@@ -2874,7 +2882,7 @@ mod tests {
28742882
assert_ne!(canonicalize_hook_path(distinct).as_ref(), Some(&hook), "{distinct:?}");
28752883
}
28762884
// A validly single-encoded literal percent (`%25` -> `%`) is DECIDABLE and
2877-
// must not fall into the fail-closed branch (regression: decode-until-stable
2885+
// must not fall into the undecidable branch (regression: decode-until-stable
28782886
// used to 403 `/reports/100%25`). It canonicalizes to a concrete route.
28792887
assert_eq!(
28802888
canonicalize_hook_path("/reports/100%25"),

0 commit comments

Comments
 (0)