Repository navigation
fix(vercel): refuse empty and dot-only identifiers in URL paths - #41
Conversation
encode_segment leaves dots unescaped, so a site or deployment id of '..' became a dot-segment and addressed the parent route; escaping the dots would not help because the URL parser decodes %2E%2E the same way. An empty id addressed the collection. encode_segment now returns Error::InvalidIdentifier for '', '.' and '..', before any request, and the doc comment no longer claims '..' protection it did not have.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(vercel): refuse empty and dot-only identifiers in URL paths. This pull request closes a path-resolution gap in the Vercel adapter: `encode_segment` left `.` unescaped, so identifiers of `..` or an empty string produced dot-segments that addressed the parent or collection route of an authenticated request. `encode_segment` now returns a new `Error::InvalidIdentifier` for `""`, `"."` and `".."`, and all nine path-construction call sites in the Vercel `Host` implementation propagate it with `?`, so nothing is sent. The `PATH_SEGMENT`/`encode_segment` doc comments are corrected, and a new sibling test file exercises the affected methods against a mock server. Review snapshot
Completeness: Complete What changedThe shared error enum gains an additive `InvalidIdentifier` variant. `encode_segment` changes from returning a displayable encoder to returning a `Result`, rejecting `""`, `"."` and `".."` before encoding. Its nine call sites in the Vercel `Host` implementation (project lookup, env set/get, deployment and events lookup, promote, add/list domains) now use `?` to propagate the error. Documentation on the escape set and encoder is updated to state that escaping alone does not prevent dot-segments, and a new `dot_segment_tests.rs` module is registered behind `#[cfg(test)]`. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
Summary
The Vercel adapter builds its request paths with
encode_segment, which percent-escapes everything outside RFC 3986's unreserved set but leaves.unescaped. So a site or deployment id of..became a dot-segment:find_site("..")requested/v9/projects/..(which resolves to/v9/), andset_env("..", …)posted to/v10/projects/../env, i.e./v10/env, an authenticated request to a different endpoint than the one named. Escaping the dots would not help: the URL parser decodes%2E%2Eback into a dot-segment. An empty id addressed the collection (/v9/projects/). The ids can come from a model's tool arguments. The doc comment onPATH_SEGMENTalso claimed the escaping prevented.., which it did not.Fix, at the one function all nine path uses go through:
encode_segmentreturnsError::InvalidIdentifierfor"","."and"..", and each call site propagates it with?, so nothing is sent.InvalidIdentifieris a new variant on the (#[non_exhaustive]) shared error enum, as the error module's guidance asks ("add a variant rather than encoding new context into an existing message"). The doc comment now says what the escaping does and does not do.Related issue
None.
API or behavior changes
Error::InvalidIdentifier("identifier must not be empty,.or.."). The enum is#[non_exhaustive], so this is additive."",.or..instead of sending the request.Validation
rustfmt --edition 2024 --checkon the touched filescargo clippy --all-targets --all-features -- -D warnings: cleancargo build --all-targets --all-features: left to CIcargo test --all-features: only the regression tests ran locally (below); CI runs the suiteCARGO_INCREMENTAL=0 cargo test --lib -- dot_only_identifiers_are_refusedFAILED. 0 passed; 1 failed.find_site("")returnedOkafter sending a request.ok. 3 passed; 0 failed(with the existinga_path_shaped_site_name_cannot_redirect_the_requestanda_missing_site_is_not_an_error).Tests
New
providers/vercel/dot_segment_tests.rs(sibling;mod_tests.rsis already long): for"","."and"..",find_site,set_env,deploymentandlist_domainseach returnInvalidIdentifier, and the mock provider receives no request.Documentation
The
PATH_SEGMENT/encode_segmentdoc comments are corrected.Checklist
#[allow(...)],#[ignore], or relaxed lints (the new test file carries the sameunwrap_used/expect_usedallow as the existing test files).envcontents in the diff or the descriptionInherited CI red
module-ci / supply-chainfails on this PR because of RustSec advisories against the existing dependency tree (cargo-deny reports RUSTSEC-2026-0346 through 0349, 0367 through 0369, 0381 and 0382), including RUSTSEC-2026-0346 (Debugimpls for client session values expose TLS secrets) and RUSTSEC-2026-0347 (buffered application data sent to the outer server after ECH rejection) inrustls. Both were published after main's last CI run, and this PR does not change dependencies. Fixing it needs arustlsbump on main.