mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 19:41:47 +08:00
fix(viewer): a file with % in its name opens as itself (#2933)
codegraph ui couldn't show a file whose name contains a %: 100%.ts was refused as an unusable path, and a%20b.ts was served as changed on disk with no lines. resolveProjectFile, the chokepoint for every file the viewer reads, percent-decoded its path. But that path is either one the index stored or a query value the URL parser had already decoded once, so this was a second decode. It threw on 100%.ts and turned a%20b.ts into a b.ts, while resolveRequestedFile looked the file up in the index by the name as given: the bytes of a b.ts were hashed against a%20b.ts's record. The chokepoint no longer decodes. It refuses raw control bytes, and on Windows a backslash, then checks containment as before. Raw request paths (the viewer's own assets) are still decoded where they are read. The File view's /api/file/<path>, whose path the HTTP layer also decodes once, had the same double decode and is fixed by the same change. The POSIX half of PR #2009 (backslashes kept in names on POSIX) is left out, as it needs a wider change and a POSIX run. Validation on Windows 11, with a fixture holding src/a%20b.ts, src/a b.ts and src/100%.ts: - codegraph ui over HTTP, /api/source with the query a browser sends: main serves a%20b.ts as drift=true with no lines and answers 403 for 100%.ts; the fix serves all three files as themselves, drift=false. - Tests: 2 new tests (the chokepoint, and /api/source plus /api/file/<path> over HTTP) fail on main's source and pass with the fix. Two existing cases that fed an encoded ..%2f and %00 straight to the chokepoint now go through the URL parser's single decode, as requests do, and are still refused. Reported in PR #2009 by @shrankr; this reimplements the viewer half of the fix independently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
983c4df374
commit
2afee00cb0
@@ -114,7 +114,9 @@ export function resolveRequestedFile(
|
||||
requested: string
|
||||
): { record: FileRecord; storedPath: string; absolute: string } {
|
||||
const posix = toRequestPath(requested);
|
||||
// Refusals happen here, ahead of everything.
|
||||
// Refusals happen here, ahead of everything. The read and the index lookup
|
||||
// below use the same string, so a `%` in a file's name means the same file
|
||||
// to both (#2009).
|
||||
const absolute = resolveProjectFile(projectRoot, posix);
|
||||
|
||||
const found = findIndexedFile(cg, posix);
|
||||
|
||||
@@ -244,6 +244,9 @@ export function resolveStaticAsset(rootDir: string, urlPath: string): string | n
|
||||
* — treating it as a separator everywhere is the safe direction, and no built
|
||||
* asset name contains one).
|
||||
*
|
||||
* For a RAW request path only. A source path handed to
|
||||
* {@link resolveProjectFile} has already been decoded.
|
||||
*
|
||||
* @returns the decoded path, or `null` if it is unusable.
|
||||
*/
|
||||
function decodePath(urlPath: string): string | null {
|
||||
@@ -258,6 +261,18 @@ function decodePath(urlPath: string): string | null {
|
||||
return decoded;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a project-relative source path is one we would read at all: no raw
|
||||
* control byte (NUL truncates a path in some syscalls), and on Windows no
|
||||
* backslash, a separator there that indexed paths never use. Nothing is
|
||||
* decoded: a `%` in the path is part of a file name.
|
||||
*/
|
||||
function isUsableSourcePath(relativePath: string): boolean {
|
||||
// eslint-disable-next-line no-control-regex -- rejecting raw control bytes IS the point
|
||||
if (/[\x00-\x1f\x7f]/.test(relativePath)) return false;
|
||||
return !(process.platform === 'win32' && relativePath.includes('\\'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a project-relative source path to an absolute path that is safe to
|
||||
* read and hand to the browser.
|
||||
@@ -268,6 +283,11 @@ function decodePath(urlPath: string): string | null {
|
||||
* what keeps `/api/source?path=../../.ssh/id_rsa` from being a credential leak
|
||||
* over a port the user opened to read their own code.
|
||||
*
|
||||
* The path is taken as it is stored in the index, or as a query value the URL
|
||||
* parser has already decoded once, and is never decoded again. Decoding it a
|
||||
* second time refused `100%.ts` and read `a b.ts` for `a%20b.ts`, while the
|
||||
* index lookup used the name as given (#2009).
|
||||
*
|
||||
* @throws {PathRefusalError} when the root is a sensitive system directory, or
|
||||
* the path escapes the root by traversal or symlink.
|
||||
*/
|
||||
@@ -275,8 +295,7 @@ export function resolveProjectFile(projectRoot: string, relativePath: string): s
|
||||
if (typeof relativePath !== 'string' || relativePath.trim() === '') {
|
||||
throw new PathRefusalError('No file path was given.');
|
||||
}
|
||||
const decoded = decodePath(relativePath);
|
||||
if (decoded === null) {
|
||||
if (!isUsableSourcePath(relativePath)) {
|
||||
throw new PathRefusalError(`Refusing to read an unusable path: ${relativePath}`);
|
||||
}
|
||||
|
||||
@@ -286,13 +305,13 @@ export function resolveProjectFile(projectRoot: string, relativePath: string): s
|
||||
const rootError = validateProjectPath(projectRoot);
|
||||
if (rootError) throw new PathRefusalError(rootError);
|
||||
|
||||
if (path.isAbsolute(decoded)) {
|
||||
throw new PathRefusalError(`Refusing to read an absolute path: ${decoded}`);
|
||||
if (path.isAbsolute(relativePath)) {
|
||||
throw new PathRefusalError(`Refusing to read an absolute path: ${relativePath}`);
|
||||
}
|
||||
|
||||
const absolute = validatePathWithinRoot(projectRoot, decoded);
|
||||
const absolute = validatePathWithinRoot(projectRoot, relativePath);
|
||||
if (!absolute) {
|
||||
throw new PathRefusalError(`Refusing to read a path outside the project: ${decoded}`);
|
||||
throw new PathRefusalError(`Refusing to read a path outside the project: ${relativePath}`);
|
||||
}
|
||||
return absolute;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user