Skip to content

fix: getRelativePath must compare Windows drive-letter/UNC paths case-insensitively#584

Open
chuenchen309 wants to merge 2 commits into
nashsu:mainfrom
chuenchen309:fix/get-relative-path-windows-case-insensitive
Open

fix: getRelativePath must compare Windows drive-letter/UNC paths case-insensitively#584
chuenchen309 wants to merge 2 commits into
nashsu:mainfrom
chuenchen309:fix/get-relative-path-windows-case-insensitive

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

Problem

getRelativePath() (src/lib/path-utils.ts) strips a base-path prefix with
a case-sensitive startsWith:

export function getRelativePath(fullPath: string, basePath: string): string {
  const normalFull = normalizePath(fullPath)
  const normalBase = normalizePath(basePath).replace(/\/$/, "")
  if (normalFull.startsWith(normalBase + "/")) {
    return normalFull.slice(normalBase.length + 1)
  }
  return normalFull
}

Windows drive-letter and UNC paths are case-insensitive at the filesystem
level, but this comparison isn't. When a recursively-enumerated file's path
casing differs from the caller's base path (e.g. a folder-picker dialog
returns "C:/Users/Me/Inbox" but a filesystem walk yields
"c:/users/me/inbox/sub/report.pdf" for a file under it), the prefix check
fails and the function falls back to returning the entire absolute path
instead of a relative one.

The real call site, importSourceFolder() in source-lifecycle.ts:322,
joins that "relative" path onto a destination directory:

const relativeSourcePath = getRelativePath(file.path, sourceRoot)
const destPath = `${destDir}/${relativeSourcePath}`
const relPath = `raw/sources/${folderName}/${relativeSourcePath}`

With the casing mismatch, destPath becomes a malformed path with an
embedded drive letter (e.g.
C:/project/raw/sources/Inbox/c:/users/me/inbox/sub/report.pdf), and the
same corrupted value feeds isPathAllowedBySourceWatch, silently defeating
the source-watch filter for every file under a casing-mismatched root.

This is the same bug class already fixed for scheduled imports in
a03c502 ("fix: preserve nested scheduled-import paths on
case-insensitive Windows roots"), which added a caseFoldPath helper to
scheduled-import.ts's own containment check. getRelativePath is a
separate, more general helper (also used by lint.ts) that never got the
same treatment.

Fix

Fold Windows drive-letter/UNC paths to lowercase for the comparison only
(mirroring scheduled-import.ts's existing caseFoldPath pattern) — the
returned path keeps its original casing. Unix paths are unaffected and
remain case-sensitive.

Testing

Added to src/lib/path-utils.test.ts:

  • drive-letter casing mismatch (c:/... vs C:/...)
  • UNC path casing mismatch (//server/SHARE vs //SERVER/share)
  • companion test confirming Unix paths stay case-sensitive

Confirmed red against unfixed code (both new casing tests failed, returning
the full absolute path instead of the relative suffix), green after the fix,
with all pre-existing getRelativePath tests (including the Unix
case-sensitivity path) still passing.

Ran source-lifecycle.test.ts + lint.test.ts (the two callers of
getRelativePath): 28 passed. Full npm run test:mocks: 115 files / 1681
tests passed, no regressions. npm run typecheck: clean.

Duplicate-work check

No open PR touches path-utils.ts; searched PR titles/bodies for
getRelativePath — no hits.


Disclosure: I used AI assistance (Claude) to find this bug and draft the
fix and tests. I independently traced the real call site, reproduced the
casing-mismatch failure against unfixed code, verified the fix red→green,
and ran the full local test suite + typecheck before submitting.

…-insensitively

getRelativePath()'s prefix check uses a case-sensitive startsWith,
but Windows drive-letter and UNC paths are case-insensitive. When a
recursively-enumerated file path's casing differs from the caller's
base path (e.g. a folder-picker returns "C:/Users/Me/Inbox" but the
filesystem walk yields "c:/users/me/inbox/sub/report.pdf"), the
prefix check fails and the function falls back to returning the
full absolute path instead of a relative one.

The real call site (importSourceFolder in source-lifecycle.ts) joins
that "relative" path onto a destination directory, producing a
malformed path with an embedded drive letter, and feeds the same
value to isPathAllowedBySourceWatch, silently defeating the
source-watch filter. This is the same bug class already fixed for
scheduled imports in a03c502 (case-insensitive containment check for
scheduledImportDestinationForFile) via a caseFoldPath helper --
getRelativePath is a separate, more general function that never
received the same fix.

Fix: fold Windows drive-letter/UNC paths to lowercase for the
comparison only (matching scheduled-import.ts's existing
caseFoldPath pattern); the returned path keeps its original casing.
Unix paths are unaffected (still case-sensitive).

TDD-verified: 2 new tests (drive-letter and UNC casing mismatch)
red before the fix, green after; added a companion test confirming
Unix paths remain case-sensitive. Full src/lib/path-utils.test.ts
(32), source-lifecycle.test.ts, and lint.test.ts (28 total) pass. Full
`npm run test:mocks` (115 files / 1681 tests) and `npm run typecheck`
pass with no regressions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chuenchen309

Copy link
Copy Markdown
Contributor Author

Windows CI job failed at the "Install protoc (Windows)" step (before the frontend build/tests ever ran) — looks like an infra/tooling flake unrelated to this diff (pure TS change to path-utils.ts). macOS/Ubuntu both passed. I don't have permission to re-run the job as an external contributor; happy to push an empty commit to retrigger if that's easier, or feel free to re-run whenever convenient.

@chuenchen309

Copy link
Copy Markdown
Contributor Author

Windows CI 的 choco install protoc 失敗是 Chocolatey 套件源暫時性連線問題(HTTP 499),跟這個 PR 的 diff 無關(純字串大小寫比對修正,不涉及 protoc/proto 相關程式碼)。我沒有權限重跑 CI,若方便的話麻煩重跑一次應該就會過。

No code change. The previous Windows CI run failed at the
"Install protoc (Windows)" step (a transient Chocolatey network
error, HTTP 499) before the frontend build/tests ever ran --
unrelated to this PR's diff. macOS/Ubuntu were already green.
Retriggering since I don't have permission to re-run the job
directly as an external contributor.
@chuenchen309

Copy link
Copy Markdown
Contributor Author

CI is green across all three platforms now (the Windows protoc install flake cleared on retry). Ready for review whenever convenient.

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