Skip to content

Fix the path and token-logging reviews JevGate got wrong on vaultwarden - #34

Merged
tauanbinato merged 3 commits into
mainfrom
rocket-paths-opt-in-logging
Sep 27, 2026
Merged

tauanbinato merged 3 commits into
mainfrom
rocket-paths-opt-in-logging

Conversation

@tauanbinato

Copy link
Copy Markdown
Contributor

The vaultwarden PR (dani-garcia/vaultwarden#7777) says that 10 of JevGate's 16 security reviews there were wrong and that those are JevGate's bugs to fix. This fixes the two classes it named:

  • Path findings on typed route parameters. Rocket's PathBuf segments, and id types whose FromParam accepts only a UUID or [A-Za-z0-9-], can't carry ... The path check only reads "a variable joined to a directory". An injection finding that rests only on a path is now asked, as a finding-only follow-up, what the path's variable parts can hold. It gets its callers and the definitions of the project's types its parameters name, with their attributes (e.g. #[derive(UuidFromParam)] pub struct CipherId(String);). If the answer leans toward names that stay inside their directory, the finding is a note, with its own message.
  • Token logs an operator opts into. A sensitive-data finding its log checks raised is now asked when the log line runs: always, debug level, or only under a setting whose purpose is that logging (off by default). The opt-in answer at the threshold makes the log signals a note, with its own message. Debug level alone still counts.

Measured on the 17 corpus projects with path or log findings, with the new follow-ups paid for (under $0.01) and reruns free:

Before After
vaultwarden path reviews (web_files, attachments, static_files_dev, download_send), labeled wrong review note (confined 0.67–0.78)
vaultwarden sso_client.rs tokens under SSO_DEBUG_TOKENS, labeled wrong review note (opt_in 0.98)
pgweb RequestLogger, labeled wrong review note (opt_in 0.89)
Path findings labeled right (javavulnlab, pygoat, railsgoat) unchanged "outside" at 0.96+
Log findings labeled right unchanged "always" or "debug"

On my patched vaultwarden branch, the sso.rs line moved under the same gate is a note too.

The path lean uses the 0.50 leaning share, as the values follow-up of injection considers does. The four wrong ones landed at 0.67–0.78 because a type's parsing is visible only by its derive name; every right one was near 0 confined.

Also: enums and the new types share one definition index, and unlocated_units delegates to locate_due (both flagged by the self-check). INJECTION and SENSITIVE_DATA rule versions are bumped.

Tests (451 + new ones for both follow-ups, each checked to fail without its rule), clippy and cargo +1.90.0 check --locked pass. The self-check reports no review or consider.

…e the security fixes

Path findings are asked what their paths can hold, with the project's
types their parameters name; leaning toward names that stay in their
directory, they are notes. Log findings are asked when the line runs; a
setting an operator turns on to log those values makes them notes.

On the 17 corpus projects with such findings: vaultwarden's 4 path
reviews on typed Rocket parameters and 2 opt-in token logs (vaultwarden,
pgweb), all labeled wrong, are notes; no finding labeled right changed.
@tauanbinato
tauanbinato merged commit f134b7a into main Sep 27, 2026
9 checks passed
@tauanbinato
tauanbinato deleted the rocket-paths-opt-in-logging branch September 27, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant