mirror of
https://github.com/ruvnet/ruflo.git
synced 2026-09-14 14:01:28 +08:00
Dragan Spiridonov
3030924f22
* fix(security): compare candidate and prefix in the same canonical form (#3010) `validate()` resolved the candidate with `fs.realpath` but the allowed prefixes were only `path.resolve()`d, so the two sides were compared in different forms and nothing under a prefix reached through a symlink could ever match. On macOS that is every temp directory — `os.tmpdir()` is `/var/folders/...` and `/var` is a symlink to `/private/var` — so `ruflo proxy install` rejected the binary it had just signature-verified. Linux is unaffected because `/tmp` is real, which is why CI stayed green. Keep both forms of every prefix and compare against the one matching how the candidate was resolved. Canonicalizing the prefixes outright would have broken `validateSync()` / `isWithinAllowed()`, which are documented as lexical-only, in the mirror direction. When the candidate's leaf does not exist, canonicalize its longest existing ancestor instead of falling back to the lexical path — otherwise `allowNonExistent` (the default) would regress under a symlinked prefix. That also closes a hole this exposed: `<allowed>/escape/not-written-yet`, where `escape` symlinks outside the prefix, was previously accepted because `realpath` threw ENOENT on the missing leaf. Escapes stay blocked, and the regression tests construct the symlink explicitly rather than relying on the macOS quirk, so they hold on every platform. 3 of the 6 fail without this change. Co-Authored-By: RuFlo <ruv@ruv.net> * fix(security): keep root prefixes usable and leave strict results untouched Three defects found by an adversarial review of the previous commit. `allowNonExistent: false` was still canonicalizing the candidate on ENOENT. The call already fails, so the only effect was rewriting the `resolvedPath` and `matchedPrefix` reported back — a silent shape change for any caller reading those off a rejected result. Canonicalize only when non-existent paths are actually allowed. A root prefix (`/`, `C:\`, a UNC share root) already ends in a separator, so anchoring the boundary as `prefix + path.sep` searched for `//` and rejected every descendant. Pre-existing at all three comparison sites — this change only widened how a prefix can end up being a root, since one can now canonicalize to it. Extracted `isWithinPrefix`, which keeps the boundary anchored so `/srv/app-secrets` is still outside `/srv/app`. The guarded symlink tests returned early instead of skipping, so they reported as passed on a host that cannot create links, and the flag was never reset between cases. They now skip properly and reset per case. Also documents that prefixes are canonicalized once, at construction: the allowlist keeps denoting what it named when configured, rather than handing whoever can rewrite a link the ability to redirect it later. Co-Authored-By: RuFlo <ruv@ruv.net> --------- Co-authored-by: RuFlo <ruv@ruv.net>