diff --git a/docs/references/file/architecture.md b/docs/references/file/architecture.md index 5e8945d913d..4c8fe2b2e74 100644 --- a/docs/references/file/architecture.md +++ b/docs/references/file/architecture.md @@ -104,7 +104,7 @@ Pure FS primitives (src/main/utils/file/) — shared raw FS primitives, open to │ atomic write: atomicWriteFile / atomicWriteIfUnchanged / createAtomicWriteStream │ version: statVersion / contentHash (xxhash-h64) ├── shell.ts — system ops: open / showInFolder -├── path.ts — path utils: resolvePath / isPathInside / canWrite / isNotEmptyDir / canonicalizeExternalPath +├── path.ts — path utils: resolvePath / isPathInside / canWrite / isNotEmptyDir ├── metadata.ts — type detection: getFileType / isTextFile / mimeToExt ├── search.ts — directory search: listDirectory (ripgrep + fuzzy matching) ├── legacyFile.ts — shared legacy helpers (`getFileType(ext)` / `sanitizeFilename` / `getAllFiles` / `pathExists` / …); planned to be split into the modules above over time @@ -299,7 +299,7 @@ All operations that can act on any file (FileEntry or arbitrary path) **accept a | Method | Description | |---|---| | `createInternalEntry` / `batchCreateInternalEntries` | Create a new Cherry-owned FileEntry (writes to `{userData}/Data/Files/{id}.{ext}`; each call produces an independent new entry, no conflict possible) | -| `ensureExternalEntry` / `batchEnsureExternalEntries` | Pure upsert by `externalPath`—the entry point first `canonicalizeExternalPath(raw)` normalizes it (see `pathResolver.ts`); reuses the existing entry with the same path or inserts a new one. Idempotent by design—callers may safely repeat calls. No "restore" branch: external entries cannot be trashed. External rows carry no stored `size` (always `null`); live values come from `getMetadata`. | +| `ensureExternalEntry` / `batchEnsureExternalEntries` | Pure upsert by `externalPath`—the entry point first validates the path shape via `FilePathSchema` (shape-only, no rewrite), then `ensureExternalEntry` canonicalizes it to the byte-faithful lexical form via the `canonicalizeFilePath()` factory (see `canonicalize.ts`) before matching; reuses the existing entry with the same path or inserts a new one. Idempotent by design—callers may safely repeat calls. No "restore" branch: external entries cannot be trashed. External rows carry no stored `size` (always `null`); live values come from `getMetadata`. | | `trash` / `restore` | Soft delete based on deletedAt (DB only). **Internal-origin only** — external-origin entries cannot be trashed (`fe_external_no_delete` CHECK); passing an external id throws. | | `batchTrash` / `batchRestore` | Batch versions of `trash` / `restore` — same internal-origin-only rule. | | `batchPermanentDelete` | Batch version of `permanentDelete`. | @@ -663,7 +663,7 @@ The File IPC adapter is a transport/dispatch layer. It may depend on FileManager | services/file/utils/* (file-module path/API helpers) | | | | Role: higher-level path-arm helpers with file-module semantics | -| Examples: resolvePhysicalPath, canonicalizeExternalPath, getMetadataByPath | +| Examples: resolvePhysicalPath, getMetadataByPath | | FS: may delegate to @main/utils/file/*; no DB lifecycle ownership | +-------------------------------------------------------------------------+ | DanglingCache (file_module singleton, not lifecycle) | diff --git a/docs/references/file/directory-tree.md b/docs/references/file/directory-tree.md index 0eea9c74a64..94253993e27 100644 --- a/docs/references/file/directory-tree.md +++ b/docs/references/file/directory-tree.md @@ -168,7 +168,7 @@ The `file:tree:*` prefix places these alongside `File_Open` / `File_Read` / etc. ### 4.2 Validation -Both `File_TreeCreate` and `File_TreeDispose` validate their payloads through Zod at the handler boundary. `rootPath` must satisfy `AbsolutePathSchema` (non-empty, no null bytes, starts with `/` or `[A-Z]:\`). `options` is validated against `DirectoryTreeOptionsSchema` — the same schema whose `z.infer` produces the `DirectoryTreeOptions` TypeScript type, so wire shape and static type cannot drift. +Both `File_TreeCreate` and `File_TreeDispose` validate their payloads through Zod at the handler boundary. `rootPath` must satisfy `FilePathSchema` (non-empty, no null bytes, starts with `/` or `[A-Z]:\`). `options` is validated against `DirectoryTreeOptionsSchema` — the same schema whose `z.infer` produces the `DirectoryTreeOptions` TypeScript type, so wire shape and static type cannot drift. A malformed payload rejects with a `ZodError` Promise rejection at the IPC boundary; the renderer's `invoke()` rejects with the same error. There is no silent narrowing — handlers never see an unvalidated object. diff --git a/docs/references/file/file-manager-architecture.md b/docs/references/file/file-manager-architecture.md index 075d8ea7d4b..2ea740da9b8 100644 --- a/docs/references/file/file-manager-architecture.md +++ b/docs/references/file/file-manager-architecture.md @@ -55,9 +55,9 @@ CREATE UNIQUE INDEX fe_external_path_lower_unique_idx `fe_external_path_idx` (plain index on the raw `external_path`) backs byte-exact lookups (`findByExternalPath`, rename re-finds, path-resolution call sites). The functional index simultaneously serves the case-insensitive lookup path (`WHERE lower(externalPath) = lower(?)`) used by `findCaseInsensitivePeers` and enforces the uniqueness invariant — `ensureExternalEntry` MUST resolve case-collisions at the application layer before INSERT (see "Duplicate-entry detection on insert" below) because a DB-level rejection would otherwise surface as an opaque `SQLITE_CONSTRAINT`. Internal rows (`externalPath = NULL`) are exempt — SQLite treats multiple NULLs as distinct in a UNIQUE index. -**Canonical invariant of `externalPath`**: SQLite performs **byte-level** comparison on the raw `externalPath` column and cannot natively detect NFC ≡ NFD (Unicode). The functional index above handles case folding via `lower()` but does **not** apply Unicode normalization, so `externalPath` **must** be normalized via `canonicalizeExternalPath(raw)` before persistence—this is an application-layer invariant, with `ensureExternalEntry` and `fileEntryService.findByExternalPath` as mandatory call sites. +**Stored form of `externalPath` — byte-faithful**: `externalPath` is persisted **exactly as the OS handed it to us**, with only reachability-safe lexical cleanup (null-byte reject, `./`/`../`/repeated-separator collapse, trailing-separator trim). It is **not** Unicode-normalized, so the stored string always reaches the real file on every filesystem — including normalization-*sensitive* ones (Linux ext4/btrfs) where an NFC-rewritten path would not exist on disk (see [Rejected: Unicode (NFC) normalization](#rejected-unicode-nfc-normalization-of-externalpath) below). Uniqueness is therefore **byte-exact plus case-fold** only: the `lower()` functional index catches identical and case-different paths; it deliberately does not fold Unicode forms. -**Compile-time enforcement via `CanonicalExternalPath` brand**: `canonicalizeExternalPath()` returns a branded `CanonicalExternalPath` (TS phantom type, zero runtime cost; see `src/shared/data/types/file/fileEntry.ts`). Every DB read/write surface that filters by `externalPath` — today `findByExternalPath`, and any future DataApi endpoint or repository method — MUST accept this type, not a plain `string`. The type system then guarantees callers routed their input through the normalization function, eliminating the "forgot to canonicalize" class of bug that would silently miss all matches. +**Compile-time enforcement via the `FilePath` brand**: `FilePathSchema.parse()` returns a branded `FilePath` (Zod brand, zero runtime cost; see `src/shared/types/file/common.ts`), and the external-entry lookup/write surfaces (`findByExternalPath`, `setExternalPathAndName`) accept the narrower `CanonicalFilePath` produced by the `canonicalizeFilePath()` factory. Every DB read/write surface that filters by `externalPath` MUST accept the branded type, not a plain `string`, so a caller cannot pass an unvalidated raw path and silently miss all matches. (`FilePathSchema` only *validates the absolute-path shape* — it does not mutate the value; see [FilePath vs CanonicalFilePath](../../../src/shared/types/file/common.ts).) | Source | Natively canonical | Relies on normalization to disambiguate | |---|---|---| @@ -67,50 +67,44 @@ CREATE UNIQUE INDEX fe_external_path_lower_unique_idx | External URL scheme / shell integration | ❌ | Same as above | | v1 migration (inherits Dexie stored values) | ❌ (inherits legacy value quality) | Canonicalize once during migration | -**Normalization scope** (synchronous, no FS IO): +**Normalization scope** (synchronous, no FS IO) — reachability-safe lexical cleanup only: - Null-byte rejection — `raw.includes('\0')` → throw, so poisoned paths never reach DB persistence (reject at the earliest boundary, not at use-time inside `resolvePhysicalPath`) - `path.resolve(raw)` → absolutize + eliminate `./` `../` -- `.normalize('NFC')` → Unicode normalization (closes the NFD/NFC window for macOS CJK) - Trailing separator trimming +- Windows: drive-letter case folding (`c:\` → `C:\`) + +Every step above is a filesystem no-op for *which file the path reaches* — the cleaned string still resolves to the same on-disk entry on every platform. **Unicode (NFC) normalization is deliberately NOT a step here** — see [Rejected](#rejected-unicode-nfc-normalization-of-externalpath) below. **Intentionally omitted** (deferred until concrete user feedback warrants the cost): -- `fs.realpath` as a step *inside* `canonicalizeExternalPath` itself (would require async FS IO at every canonicalization call site and a file-existence precondition). `fs.realpath` IS used on the `ensureExternalEntry` collision path described below — that is a per-collision probe, not a per-canonicalize step. +- `fs.realpath` as a step *inside* `canonicalizeAbsolutePath` itself (would require async FS IO at every call site and a file-existence precondition). `fs.realpath` IS used on the `ensureExternalEntry` collision path described below — that is a per-collision probe, not a per-canonicalize step. - Symlink target merging at canonicalize time - Windows 8.3 short-name resolution -See the JSDoc for `canonicalizeExternalPath` in `src/main/services/file/utils/pathResolver.ts` for the detailed contract. - -#### Rule evolution discipline - -Because the canonical form is **application-layer logic**, not DB schema, any change to `canonicalizeExternalPath`'s normalization steps desynchronizes historical rows (written under the old rule) from new queries (running under the new rule). This produces a silent failure mode: byte-compare misses, the user sees "my file is in the library but the app says it isn't", and `ensureExternalEntry` inserts a duplicate. - -**Rule**: modifying `canonicalizeExternalPath` ≡ ship a paired Drizzle migration that re-canonicalizes every existing `file_entry` row with `origin='external'` in the **same PR**. No exceptions — even if the new rule is claimed "strictly more permissive", the byte-compare will still miss. +See the JSDoc for `canonicalizeAbsolutePath` in `src/shared/utils/file/canonicalize.ts` for the detailed contract. -When a rule change additionally collapses previously-distinct strings to the same canonical form (e.g. adding `fs.realpath` merges APFS case-insensitive duplicates), the migration MUST also merge the colliding rows. The rules below are prescriptive; follow them exactly rather than improvising per-migration. +#### Rejected: Unicode (NFC) normalization of `externalPath` -**Winner selection when merging rows**: +An earlier design NFC-normalized `externalPath` (inside `canonicalizeAbsolutePath`) before persistence, so a file named with a decomposable character (`é` = `e` + combining acute vs the precomposed `é`) would dedup to one row on macOS. **This is deliberately rejected.** `externalPath` is stored byte-faithful (see "Stored form" above) and is never Unicode-normalized. -1. Oldest `createdAt` wins (preserves user-visible history — a 3-year-old entry's creation timestamp is more valuable than a 3-day-old one's). -2. Tiebreaker: highest ref count (keeps the entry that more of the user's data already points at). -3. Final tiebreaker: smallest `id` by lexicographic order (deterministic, no FS-state dependency). +1. **Reachability (blocking).** On a normalization-*sensitive* filesystem (Linux ext4/btrfs, some SMB/NFS mounts) the directory entry stores exact bytes. NFC-normalizing an NFD-named file's path yields a string that does **not** exist on disk, so `ensureExternalEntry`'s `fs.stat`, the copy source, and `resolvePhysicalPath` all `ENOENT` — the file becomes unimportable and unreachable. Breaking Linux functionality to dedup on macOS is not an acceptable trade. +2. **Unverified premise.** The "most common duplicate trigger for CJK users" justification was AI-authored in a comment; this v2 code has never shipped, and there is no observed evidence that this app's path sources (`showOpenDialog`, drag-drop, `fs.readdir`) ever surface the same file in divergent normalization forms. +3. **Rare and cosmetic even on macOS.** A duplicate row needs a conjunction: the same file added more than once, through two ingestion routes that *disagree* on normalization, with a decomposable non-ASCII name, stored on disk in a divergent form. Even then APFS is normalization-insensitive, so both rows resolve to the *same* physical file — the harm is one extra, user-deletable list entry, not data loss or breakage. And the `fs.realpath` collision probe below does not catch it (NFD vs NFC are not `lower()` case-peers), so NFC-in-storage was the *sole* mechanism folding it — bought at the cost of item 1. +4. **YAGNI.** Storage-time normalization plus its rule-evolution migration discipline is real, permanent complexity for an unverified, rare, cosmetic condition. -**Losers' dependents** (executed in the same Drizzle transaction as the merge): +If real NFD/NFC duplicates are ever *observed*, the correct fix is an FS-aware `fs.realpath` fold at the collision probe (run only when the file exists), **never** re-introducing Unicode normalization into `canonicalizeAbsolutePath` or the stored form. -- Association rows with `fileEntryId = loser.id` → update to `winner.id`. No deduplication inside each table's `UNIQUE(fileEntryId, sourceId, role)` constraint is expected because each `(sourceId, role)` pair originally referenced only one entry; if violations occur, the update conflicts and the migration fails loudly (do not silently `ON CONFLICT DO NOTHING` — investigate). -- `file_entry.id = loser.id` → delete. -- Any downstream consumer of `loser.id` (future `file_upload.fileEntryId`, business-service caches keyed by entryId) MUST be enumerated and updated in the same migration. If you add a new table that references `file_entry.id`, the canonicalization migration procedure expands — document the expansion alongside the table's schema. - -**Atomicity**: the entire re-canonicalize + merge operation runs in one Drizzle migration transaction. On failure the DB rolls back to the pre-migration state and the next startup re-attempts; partial progress is not possible. - -**Renderer-side cache invalidation**: after the migration runs, some React Query caches keyed by the loser's `id` may be stale. Because migrations execute before the renderer boots, this is self-healing on the first query — no special coordination required. +> **Residual normalization discipline.** What remains in `canonicalizeAbsolutePath` is the reachability-safe lexical cleanup listed under "Normalization scope". It does not depend on filesystem semantics, so it does not carry the "changing the rule desyncs historical rows, requiring a paired re-canonicalize migration" hazard that the rejected NFC step did. (v2 has no shipped data to migrate regardless — schemas and rows are pre-release throwaway.) #### Duplicate-entry detection on insert Case-insensitive uniqueness on `externalPath` is enforced at **both layers**: the functional UNIQUE index `fe_external_path_lower_unique_idx` (DB) and `ensureExternalEntry`'s pre-INSERT collision check (application). The two-layer scheme keeps the DB-level guarantee unbreakable while letting the application disambiguate the FS-correct interpretation case-by-case. +Presence is **probed, not required**: after the `findByExternalPath` miss, `ensureExternalEntry` runs one `fs.stat`. `ENOENT` / `ENOTDIR` ("the file is simply not there") does **not** abort — the entry is created **dangling** (an external ref to an off-disk path is a first-class state; see [§3.3 Dangling Model](#33-dangling-model)), seeding a `'missing'` DanglingCache observation. Only a *genuine* FS fault (`EACCES`, `EIO`, …) rethrows. The `fs.realpath` collision probe below **requires the file on disk**, so it runs **only when the file is present**; when the new path is missing the block is skipped and a case-colliding missing path is left to the DB `lower(externalPath)` UNIQUE index to reject at INSERT (`SQLITE_CONSTRAINT`) — the correct outcome, just without the friendlier pre-INSERT error we cannot produce without the file. + ```typescript // Inside ensureExternalEntry, AFTER canonicalize, AFTER findByExternalPath miss, -// AFTER fs.stat verifies the new path exists, BEFORE INSERT: +// AFTER the fs.stat presence probe reports the new path is present (a MISSING +// path skips this block and is created dangling), BEFORE INSERT: const peers = await fileEntryService.findCaseInsensitivePeers(canonicalPath) if (peers.length > 0) { // `fs.realpath` is the platform-correct probe for "are these the same FS @@ -683,8 +677,8 @@ Query: `WHERE deletedAt < now() - retentionMs` → batch permanentDelete. |---|---| | unlink fails on permanentDelete internal (file already missing, permission issue) | Log warn; the DB row is already gone, so the failure surfaces only as an orphan blob that the next user-triggered orphan sweep will reclaim | | permanentDelete on external | DB-only by design; the user's file at `externalPath` is never touched — Cherry owns only the reference | -| `ensureExternalEntry(path)` when an entry for the same path already exists | Entry point first calls `canonicalizeExternalPath(raw)`; upsert returns the existing row. External entries cannot be trashed, so there is no "restore" branch. | -| **Two entries for the same file due to case / NFC differences** (macOS APFS, Windows NTFS, or NFD ↔ NFC input) | NFC closed by `canonicalizeExternalPath`; case-collision rejected at INSERT by the DB functional unique index plus the `fs.realpath`-based reuse-or-throw decision in `ensureExternalEntry` (see §1.2 "Duplicate-entry detection on insert"). | +| `ensureExternalEntry(path)` when an entry for the same path already exists | Entry point first calls `FilePathSchema.parse(raw)`; upsert returns the existing row. External entries cannot be trashed, so there is no "restore" branch. | +| **Two entries for the same file** | **Case** differences (macOS APFS, Windows NTFS): rejected at INSERT by the DB `lower()` functional unique index plus the `fs.realpath`-based reuse-or-throw probe in `ensureExternalEntry` (see §1.2 "Duplicate-entry detection on insert"). **Unicode (NFD ↔ NFC)** differences: deliberately **not** deduped — `externalPath` is stored byte-faithful, and the rare, cosmetic macOS case is accepted rather than break reachability on Linux (see §1.2 "Rejected: Unicode (NFC) normalization of `externalPath`"). | | External file at original path externally replaced with a different file | Cherry does not check content consistency (best-effort). `name` / `ext` on the row are derived from `externalPath` and do not change; `size` is always served live by `getMetadata`. DanglingCache flips to `'present'` on the next stat, so the UI just renders the new file under the existing reference. | | A trashed entry is permanently externally deleted and then restored | Appears dangling (DanglingCache returns missing on next check), UI shows failed style | | External write with permission error / disk full on target path | Throw without polluting DB; caller decides retry or user notification | @@ -1208,7 +1202,9 @@ Business modules **need not be directly aware of DanglingCache**. All watchers m - `unlink` → cache marks `missing` - `change` → cache untouched (file is still present; mtime drift is not tracked here) -The cache feed is keyed by canonical (NFC) form so it lines up with the reverse index populated by `ensureExternalEntry`; the path forwarded to subscribers is the raw OS form chokidar saw, so a subscriber that opens the file with that string stays coherent with what the FS actually has. +Because `externalPath` is stored **byte-faithful** (see §1.2 "Stored form"), the reverse index is keyed by that exact stored form and the watcher matches chokidar events by **raw byte equality** — no NFC normalization on either leg. (The NFC step that used to sit in the watcher existed only to bridge to the old NFC-canonical keys; it is removed together with them — see §1.2 "Rejected: Unicode (NFC) normalization".) The path forwarded to subscribers is likewise the raw OS form chokidar saw. + +`check()` is the correctness baseline: it stats `entry.externalPath` (byte-faithful) directly, so an entry's dangling state is always *eventually* correct regardless of whether any watcher event matched — the watcher leg is an eager-update latency optimization on top of it, never a correctness dependency. On Linux the raw event byte-matches the stored key by construction; on macOS/Windows it matches when the path source and chokidar agree on Unicode form. If they ever diverge (unverified — same open question as §1.2 "Rejected"), the only effect is that a watcher event misses and the badge stays stale until the next `check()` — benign and self-healing. The platform-conditional fold that could bridge such a divergence, *if ever observed*, belongs on the reverse-index comparison key (`process.platform`-aware: NFC on darwin/win32, identity on linux) — never on the stored form. **Note**: watcher rename events **do not auto-update an external entry's externalPath**—Cherry does not track external rename. After a rename, the original entry goes dangling; the user must re-@ to establish a new reference. @@ -1375,7 +1371,7 @@ This checklist is the canonical addition procedure. A PR introducing a new origi | Location | Change required | |---|---| | `src/main/services/file/utils/pathResolver.ts` → `resolvePhysicalPath` | Add the new `entry.origin` branch; decide storage layout | -| Same file → `canonicalizeExternalPath` | If the new variant is path-based and distinct from `'external'`, decide whether it shares the canonical form or needs its own normalization + brand | +| `src/shared/utils/file/canonicalize.ts` → `canonicalizeAbsolutePath` (via the `canonicalizeFilePath()` factory) | If the new variant is path-based and distinct from `'external'`, decide whether it shares the canonical form or needs its own normalization + brand | ### 13.4 Behavior Policy Matrix diff --git a/eslint.config.mjs b/eslint.config.mjs index 2275f0f82cd..a8124bfa1f8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -435,6 +435,64 @@ export default defineConfig([ ] } }, + // Path brand integrity — `as FilePath` / `as CanonicalFilePath` forge the + // brands, skipping the validation each asserts. `FilePath` asserts shape + // validation (build via FilePathSchema.parse); `CanonicalFilePath` asserts + // the byte-faithful lexical form that backs the external-path dedup key + // (build via the canonicalizeFilePath() factory). A forged + // `as CanonicalFilePath` silently bypasses canonicalization and can write a + // ghost-duplicate key, so the stronger brand is guarded too. + // Exemptions: test fixtures; and one deliberate raw-OS-path regime — the + // tree builder (tree/**) holds raw chokidar/OS event paths that are compared + // byte-for-byte against event paths and are trusted as-is, so they `as FilePath` + // rather than routing through validation. + { + files: ['src/**/*.{ts,tsx}'], + ignores: [ + 'src/main/services/file/tree/**', + 'src/**/__tests__/**', + 'src/**/__mocks__/**', + 'src/**/*.test.*' + ], + plugins: { + 'filepath-brand': { + rules: { + 'no-as-filepath': { + meta: { + type: 'problem', + docs: { + description: + 'Disallow `as FilePath` / `as CanonicalFilePath` casts. Both are Zod-derived brands: FilePath asserts shape validation (absolute path, no null bytes), CanonicalFilePath additionally asserts the byte-faithful lexical form backing the dedup key. Forging either bypasses its validation. Construct via FilePathSchema.parse() / canonicalizeFilePath().', + recommended: true + }, + messages: { + noAsFilePath: + '`as FilePath` forges the brand, skipping FilePathSchema\'s absolute-path validation. Build it with FilePathSchema.parse(value) instead. If this is a deliberate raw-path regime, move it under an exempted path or justify with an eslint-disable + reason.', + noAsCanonicalFilePath: + '`as CanonicalFilePath` forges the brand, skipping canonicalization — a non-canonical value silently becomes a ghost-duplicate dedup key. Build it with canonicalizeFilePath(value) instead, or justify the sanctioned producer with an eslint-disable + reason.' + } + }, + create(context) { + return { + TSAsExpression(node) { + const ann = node.typeAnnotation + if (ann?.type !== 'TSTypeReference' || ann.typeName?.type !== 'Identifier') return + if (ann.typeName.name === 'FilePath') { + context.report({ node, messageId: 'noAsFilePath' }) + } else if (ann.typeName.name === 'CanonicalFilePath') { + context.report({ node, messageId: 'noAsCanonicalFilePath' }) + } + } + } + } + } + } + } + }, + rules: { + 'filepath-brand/no-as-filepath': process.env.CI ? 'error' : 'warn' + } + }, // Application lifecycle - all quit-related APIs and events are managed by Application.ts { files: ['src/main/**/*.{ts,tsx,js,jsx}'], diff --git a/src/main/ai/messages/fileProcessor.ts b/src/main/ai/messages/fileProcessor.ts index 70a66a54a58..001cbba3906 100644 --- a/src/main/ai/messages/fileProcessor.ts +++ b/src/main/ai/messages/fileProcessor.ts @@ -22,7 +22,7 @@ import { loggerService } from '@logger' import { read as fsRead } from '@main/utils/file' import type { FileUIPart } from '@shared/data/types/message' import { readCherryMeta } from '@shared/data/types/uiParts' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' const logger = loggerService.withContext('ai:fileProcessor') @@ -51,7 +51,7 @@ async function fileEntryIdToDataUrl(fileEntryId: string) { */ async function fileUrlToDataUrl(fileUrl: string) { try { - const absPath = fileURLToPath(fileUrl) as FilePath + const absPath = FilePathSchema.parse(fileURLToPath(fileUrl)) const { data, mime } = await fsRead(absPath, { encoding: 'base64' }) return { url: `data:${mime};base64,${data}`, mediaType: mime } } catch (error) { diff --git a/src/main/data/migration/v2/migrators/KnowledgeMigrator.ts b/src/main/data/migration/v2/migrators/KnowledgeMigrator.ts index f6850abe22c..5355f396983 100644 --- a/src/main/data/migration/v2/migrators/KnowledgeMigrator.ts +++ b/src/main/data/migration/v2/migrators/KnowledgeMigrator.ts @@ -19,7 +19,7 @@ import { } from '@shared/data/types/knowledge' import type { FileMetadata } from '@shared/data/types/legacyFile' import { UNIQUE_MODEL_ID_SEPARATOR, type UniqueModelId } from '@shared/data/types/model' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import Database from 'better-sqlite3' import { eq, sql } from 'drizzle-orm' @@ -1053,8 +1053,8 @@ export class KnowledgeMigrator extends BaseMigrator { const destPath = path.join(ctx.paths.knowledgeBaseDir, baseId, 'raw', relativePath) try { - await ensureDir(path.dirname(destPath) as FilePath) - await copy(sourcePath as FilePath, destPath as FilePath) + await ensureDir(FilePathSchema.parse(path.dirname(destPath))) + await copy(FilePathSchema.parse(sourcePath), FilePathSchema.parse(destPath)) } catch (error) { this.recordWarning( `Failed to copy knowledge file for item ${item.id} (${sourcePath} → ${destPath}): ${ diff --git a/src/main/data/migration/v2/migrators/mappings/ChatMappings.ts b/src/main/data/migration/v2/migrators/mappings/ChatMappings.ts index 678d683a19f..fde17c0ff08 100644 --- a/src/main/data/migration/v2/migrators/mappings/ChatMappings.ts +++ b/src/main/data/migration/v2/migrators/mappings/ChatMappings.ts @@ -64,7 +64,7 @@ import type { } from '@shared/data/types/message' import type { CherryDataPartTypes, CherryToolMeta } from '@shared/data/types/uiParts' import { withCherryMeta } from '@shared/data/types/uiParts' -import type { Base64String, FilePath } from '@shared/types/file' +import { type Base64String, FilePathSchema } from '@shared/types/file' import type { SourceUrlUIPart } from 'ai' import mime from 'mime' import { v7 as uuidv7 } from 'uuid' @@ -953,7 +953,7 @@ async function promoteBase64ToFileEntry( const ext = mime.getExtension(mimeType) const id = uuidv7() const filename = ext ? `${id}.${ext}` : id - const physicalPath = path.join(filesDataDir, filename) as FilePath + const physicalPath = FilePathSchema.parse(path.join(filesDataDir, filename)) const bytes = Buffer.from(payload, 'base64') let physicalWritten = false diff --git a/src/main/data/services/FileEntryService.ts b/src/main/data/services/FileEntryService.ts index 8c57408c551..e892ef6af51 100644 --- a/src/main/data/services/FileEntryService.ts +++ b/src/main/data/services/FileEntryService.ts @@ -29,15 +29,16 @@ import type { DbOrTx } from '@data/db/types' import { loggerService } from '@logger' import { DataApiErrorFactory } from '@shared/data/api/errors' import type { FileEntryListResponse, FileEntryStats } from '@shared/data/api/schemas/files' -import type { CanonicalExternalPath, FileEntry, FileEntryId, FileEntryOrigin } from '@shared/data/types/file' +import type { FileEntry, FileEntryId, FileEntryOrigin } from '@shared/data/types/file' import { - AbsolutePathSchema, + chatMessageSourceType, ExternalEntrySchema, FileEntrySchema, InternalEntrySchema, + paintingSourceType, SafeNameSchema } from '@shared/data/types/file' -import { chatMessageSourceType, paintingSourceType } from '@shared/data/types/file' +import type { CanonicalFilePath } from '@shared/utils/file' import { and, asc, count, eq, isNotNull, isNull, type SQL, sql, type SQLWrapper } from 'drizzle-orm' import { v7 as uuidv7 } from 'uuid' import * as z from 'zod' @@ -119,12 +120,12 @@ export interface FileEntryService { /** * Look up an external entry by canonical `externalPath`. Returns `null` when - * no row matches. The `CanonicalExternalPath` brand forces callers through - * `canonicalizeExternalPath()` at compile time — raw `string` values are - * not assignable here, which prevents the "caller forgot to canonicalize" - * class of bug that would silently miss all matches. + * no row matches. The `CanonicalFilePath` brand forces callers through + * `canonicalizeFilePath()` at compile time — raw `string` / bare `FilePath` + * values are not assignable here, which prevents the "caller forgot to + * canonicalize" class of bug that would silently miss all matches. */ - findByExternalPath(canonicalPath: CanonicalExternalPath): FileEntry | null + findByExternalPath(canonicalPath: CanonicalFilePath): FileEntry | null /** * Return external entries whose `externalPath` matches `canonicalPath` @@ -152,7 +153,7 @@ export interface FileEntryService { * * Un-parseable rows are skipped with a warning (see `rowToFileEntrySafe`). */ - findCaseInsensitivePeers(canonicalPath: CanonicalExternalPath): FileEntry[] + findCaseInsensitivePeers(canonicalPath: CanonicalFilePath): FileEntry[] /** * Flat listing. Trashed filter defaults to "active only" when `inTrash` is omitted. @@ -222,10 +223,10 @@ export interface FileEntryService { * doing it as a single statement keeps the (path, name) pair consistent under * partial-failure scenarios (transient lock, schema constraint). */ - setExternalPathAndName(id: FileEntryId, externalPath: CanonicalExternalPath, name: string): FileEntry + setExternalPathAndName(id: FileEntryId, externalPath: CanonicalFilePath, name: string): FileEntry /** Tx-scoped variant of `setExternalPathAndName` for composing write flows. */ - setExternalPathAndNameTx(tx: DbOrTx, id: FileEntryId, externalPath: CanonicalExternalPath, name: string): FileEntry + setExternalPathAndNameTx(tx: DbOrTx, id: FileEntryId, externalPath: CanonicalFilePath, name: string): FileEntry /** Remove the row (CASCADE drops dependent persistent file refs). No-op if already gone. */ delete(id: FileEntryId): void @@ -392,7 +393,7 @@ class FileEntryServiceImpl implements FileEntryService { return entry } - findByExternalPath(canonicalPath: CanonicalExternalPath): FileEntry | null { + findByExternalPath(canonicalPath: CanonicalFilePath): FileEntry | null { const rows = this.getDb() .select() .from(fileEntryTable) @@ -402,7 +403,7 @@ class FileEntryServiceImpl implements FileEntryService { return rows.length === 0 ? null : rowToFileEntry(rows[0]) } - findCaseInsensitivePeers(canonicalPath: CanonicalExternalPath): FileEntry[] { + findCaseInsensitivePeers(canonicalPath: CanonicalFilePath): FileEntry[] { const rows = this.getDb() .select() .from(fileEntryTable) @@ -609,19 +610,24 @@ class FileEntryServiceImpl implements FileEntryService { * the only sanctioned mutation site for `externalPath`. Used by the rename * flow so the (path, name) pair stays consistent under failure. */ - setExternalPathAndName(id: FileEntryId, externalPath: CanonicalExternalPath, name: string): FileEntry { + setExternalPathAndName(id: FileEntryId, externalPath: CanonicalFilePath, name: string): FileEntry { return this.setExternalPathAndNameTx(this.getDb(), id, externalPath, name) } - setExternalPathAndNameTx(tx: DbOrTx, id: FileEntryId, externalPath: CanonicalExternalPath, name: string): FileEntry { - // Same pre-SQL validation rationale as `update` above; an unsafe value - // for either column would corrupt the row past `rowToFileEntry` parse. - // The `CanonicalExternalPath` brand is TS-only — defense-in-depth at the - // runtime layer rejects path strings the brand failed to flag (e.g. a - // caller that `as`-cast a raw user string instead of going through - // `canonicalizeExternalPath`). + setExternalPathAndNameTx(tx: DbOrTx, id: FileEntryId, externalPath: CanonicalFilePath, name: string): FileEntry { + // Same pre-SQL validation rationale as `update` above; an unsafe `name` + // would corrupt the row past `rowToFileEntry` parse. `externalPath` needs + // no full re-canonicalization here: the `CanonicalFilePath` brand can only + // be produced by `canonicalizeFilePath()`, so callers already proved + // canonicalization at construction time. The null-byte check below is + // cheap defense-in-depth against a forged `as CanonicalFilePath` cast + // (e.g. from the lint-exempt `watcher/**` / `tree/**` regimes) that would + // otherwise persist a poison value SQLite happily stores but that later + // makes the row unreadable once `canonicalizeAbsolutePath` throws on it. SafeNameSchema.parse(name) - AbsolutePathSchema.parse(externalPath) + if (externalPath.includes('\0')) { + throw new Error('setExternalPathAndNameTx: externalPath contains a null byte') + } const rows = tx .update(fileEntryTable) .set({ externalPath, name, updatedAt: Date.now() }) diff --git a/src/main/data/services/__tests__/FileEntryService.test.ts b/src/main/data/services/__tests__/FileEntryService.test.ts index ed0a50cc561..a2c94c29e3d 100644 --- a/src/main/data/services/__tests__/FileEntryService.test.ts +++ b/src/main/data/services/__tests__/FileEntryService.test.ts @@ -4,7 +4,9 @@ import { messageTable } from '@data/db/schemas/message' import { paintingTable } from '@data/db/schemas/painting' import { topicTable } from '@data/db/schemas/topic' import { DataApiError, ErrorCode } from '@shared/data/api/errors' -import type { CanonicalExternalPath, FileEntryId } from '@shared/data/types/file' +import type { FileEntryId } from '@shared/data/types/file' +import type { FilePath } from '@shared/types/file' +import type { CanonicalFilePath } from '@shared/utils/file' import { setupTestDatabase } from '@test-helpers/db' import { MockMainDbServiceExport, MockMainDbServiceUtils } from '@test-mocks/main/DbService' import { mockMainLoggerService } from '@test-mocks/MainLoggerService' @@ -117,13 +119,13 @@ describe('FileEntryService', () => { updatedAt: now }) - const entry = fileEntryService.findByExternalPath('/Users/me/doc.pdf' as CanonicalExternalPath) + const entry = fileEntryService.findByExternalPath('/Users/me/doc.pdf' as CanonicalFilePath) expect(entry?.id).toBe(id) expect(entry?.origin).toBe('external') }) it('returns null when no row matches', async () => { - const result = fileEntryService.findByExternalPath('/Users/me/nonexistent.pdf' as CanonicalExternalPath) + const result = fileEntryService.findByExternalPath('/Users/me/nonexistent.pdf' as CanonicalFilePath) expect(result).toBeNull() }) @@ -142,7 +144,7 @@ describe('FileEntryService', () => { updatedAt: now }) - const result = fileEntryService.findByExternalPath('/Users/me/A.TXT' as CanonicalExternalPath) + const result = fileEntryService.findByExternalPath('/Users/me/A.TXT' as CanonicalFilePath) expect(result).toBeNull() }) }) @@ -167,13 +169,13 @@ describe('FileEntryService', () => { updatedAt: now }) - const peers = fileEntryService.findCaseInsensitivePeers('/Users/me/a.txt' as CanonicalExternalPath) + const peers = fileEntryService.findCaseInsensitivePeers('/Users/me/a.txt' as CanonicalFilePath) expect(peers).toHaveLength(1) expect(peers[0]?.id).toBe('019606a0-0000-7000-8000-000000000020') }) it('returns empty array when no rows match', async () => { - const peers = fileEntryService.findCaseInsensitivePeers('/zzz/none.txt' as CanonicalExternalPath) + const peers = fileEntryService.findCaseInsensitivePeers('/zzz/none.txt' as CanonicalFilePath) expect(peers).toEqual([]) }) @@ -1178,7 +1180,7 @@ describe('FileEntryService', () => { const updated = fileEntryService.setExternalPathAndName( id, - '/Users/me/new-doc.pdf' as CanonicalExternalPath, + '/Users/me/new-doc.pdf' as CanonicalFilePath, 'new-doc' ) @@ -1199,7 +1201,7 @@ describe('FileEntryService', () => { const missing = '019606a0-0000-7000-8000-000000000dff' as FileEntryId let err: unknown try { - fileEntryService.setExternalPathAndName(missing, '/Users/me/ghost.pdf' as CanonicalExternalPath, 'ghost') + fileEntryService.setExternalPathAndName(missing, '/Users/me/ghost.pdf' as CanonicalFilePath, 'ghost') } catch (e) { err = e } @@ -1222,7 +1224,7 @@ describe('FileEntryService', () => { }) expect(() => - fileEntryService.setExternalPathAndName(entry.id, '/Users/me/legit.txt' as CanonicalExternalPath, '../evil') + fileEntryService.setExternalPathAndName(entry.id, '/Users/me/legit.txt' as CanonicalFilePath, '../evil') ).toThrow() const [raw] = await dbh.db.select().from(fileEntryTable).where(eq(fileEntryTable.id, entry.id)) @@ -1231,10 +1233,11 @@ describe('FileEntryService', () => { }) it('rejects unsafe externalPath BEFORE the SQL UPDATE commits', async () => { - // The `CanonicalExternalPath` brand is TS-only and offers no runtime - // guarantee. The service-side `AbsolutePathSchema.parse(externalPath)` - // catches null bytes / non-absolute paths regardless of whether the - // caller went through `canonicalizeExternalPath` or `as`-cast. + // Defense-in-depth guard: the `CanonicalFilePath` brand normally + // guarantees canonical-ness, but a forged `as CanonicalFilePath` cast + // (e.g. from the lint-exempt watcher/tree regimes) could smuggle a null + // byte past the type system. The null-byte check must reject it before + // the SQL UPDATE commits — raw SELECT proves the row stayed unchanged. const entry = fileEntryService.create({ origin: 'external', name: 'safe', @@ -1243,7 +1246,7 @@ describe('FileEntryService', () => { }) expect(() => - fileEntryService.setExternalPathAndName(entry.id, '/Users/me/null\0byte.txt' as CanonicalExternalPath, 'fine') + fileEntryService.setExternalPathAndName(entry.id, '/Users/me/legit\0.txt' as CanonicalFilePath, 'legit') ).toThrow() const [raw] = await dbh.db.select().from(fileEntryTable).where(eq(fileEntryTable.id, entry.id)) @@ -1276,7 +1279,7 @@ describe('FileEntryService', () => { // unexpected and bubble it up. let err: Error | null = null try { - fileEntryService.setExternalPathAndName(b.id, '/Users/me/a.txt' as CanonicalExternalPath, 'a') + fileEntryService.setExternalPathAndName(b.id, '/Users/me/a.txt' as CanonicalFilePath, 'a') } catch (e) { err = e as Error } @@ -1307,7 +1310,7 @@ describe('FileEntryService', () => { }) fileEntryService.setExternalPathAndName( external.id, - '/Users/me/ext-tx-renamed.txt' as CanonicalExternalPath, + '/Users/me/ext-tx-renamed.txt' as CanonicalFilePath, 'ext-tx-renamed' ) @@ -1477,7 +1480,7 @@ describe('FileEntryService', () => { origin: 'external', name: 'e', ext: 'txt', - externalPath: '/abs/orphan.txt' as CanonicalExternalPath + externalPath: '/abs/orphan.txt' as FilePath }) const externalsOnly = fileEntryService.findUnreferenced({ origin: 'external' }) @@ -1606,7 +1609,7 @@ describe('FileEntryService', () => { mockMainLoggerService.warn.mockClear() // Corrupt match → excluded with one warning, not a throw. - const badPeers = fileEntryService.findCaseInsensitivePeers('/users/me/bad-peer.txt' as CanonicalExternalPath) + const badPeers = fileEntryService.findCaseInsensitivePeers('/users/me/bad-peer.txt' as CanonicalFilePath) expect(badPeers).toEqual([]) expect(mockMainLoggerService.warn).toHaveBeenCalledTimes(1) expect(mockMainLoggerService.warn).toHaveBeenCalledWith( @@ -1615,8 +1618,76 @@ describe('FileEntryService', () => { ) // Good rows still surface through the same method. - const goodPeers = fileEntryService.findCaseInsensitivePeers('/users/me/good-peer.txt' as CanonicalExternalPath) + const goodPeers = fileEntryService.findCaseInsensitivePeers('/users/me/good-peer.txt' as CanonicalFilePath) expect(goodPeers.map((e) => e.id)).toEqual([goodExternalId]) }) }) + + describe('externalPath byte-faithful acceptance + lexical rejection (read-time contract)', () => { + it('accepts an external row whose stored externalPath carries NFD Unicode (byte-faithful)', async () => { + // externalPath is stored byte-faithful — canonicalizeAbsolutePath does NOT + // Unicode-normalize, so an NFD path IS already in canonical form and reads + // back cleanly. (NFC-folding here would break reachability on + // normalization-sensitive filesystems like Linux ext4.) ASCII \u escapes + // keep the literal stable — raw combining marks get re-normalized by tooling. + const id = '019606a0-0000-7000-8000-00000000ab01' as FileEntryId + const now = Date.now() + // "café.pdf" written in NFD: base 'e' + combining acute accent (U+0301). + const nfdPath = '/Users/me/cafe\u0301.pdf' + await dbh.db.insert(fileEntryTable).values({ + id, + origin: 'external', + name: 'cafe', + ext: 'pdf', + size: null, + externalPath: nfdPath, + deletedAt: null, + createdAt: now, + updatedAt: now + }) + mockMainLoggerService.warn.mockClear() + + // Bulk read returns the row unchanged — no warn, no silent rewrite. + const entries = fileEntryService.findMany({ origin: 'external' }) + expect(entries.map((e) => e.id)).toEqual([id]) + expect(entries[0].origin).toBe('external') + if (entries[0].origin === 'external') { + expect(entries[0].externalPath).toBe(nfdPath) // byte-faithful, still NFD + } + expect(mockMainLoggerService.warn).not.toHaveBeenCalled() + }) + + it('warn-skips an external row whose stored externalPath is not in lexical canonical form (unresolved `..`)', async () => { + // The byte-faithful refine still rejects a stored value that is not in the + // lexically-resolved form `canonicalizeFilePath` produces — no silent + // repair; lookup-by-path requires a re-canonicalization migration. A row + // carrying an unresolved `/../` segment (canonicalizeAbsolutePath collapses + // it) fails the schema's canonical-equivalence refine and is warn-skipped + // by rowToFileEntrySafe rather than returned or silently rewritten. + const id = '019606a0-0000-7000-8000-00000000ab02' as FileEntryId + const now = Date.now() + const nonCanonicalPath = '/Users/me/../me/DOC.pdf' // canonicalizes to /Users/me/DOC.pdf + await dbh.db.insert(fileEntryTable).values({ + id, + origin: 'external', + name: 'DOC', + ext: 'pdf', + size: null, + externalPath: nonCanonicalPath, + deletedAt: null, + createdAt: now, + updatedAt: now + }) + mockMainLoggerService.warn.mockClear() + + // Bulk read drops the row (fault isolation) and warns once with its id. + const entries = fileEntryService.findMany({ origin: 'external' }) + expect(entries).toEqual([]) + expect(mockMainLoggerService.warn).toHaveBeenCalledTimes(1) + expect(mockMainLoggerService.warn).toHaveBeenCalledWith( + expect.stringContaining('un-parseable'), + expect.objectContaining({ id }) + ) + }) + }) }) diff --git a/src/main/features/fileProcessing/__tests__/FileProcessingService.integration.test.ts b/src/main/features/fileProcessing/__tests__/FileProcessingService.integration.test.ts index f91e785cac8..efa0b608715 100644 --- a/src/main/features/fileProcessing/__tests__/FileProcessingService.integration.test.ts +++ b/src/main/features/fileProcessing/__tests__/FileProcessingService.integration.test.ts @@ -10,6 +10,7 @@ import type * as LifecycleModule from '@main/core/lifecycle' import { getDependencies, getPhase } from '@main/core/lifecycle/decorators' import { Phase } from '@main/core/lifecycle/types' +import type { FilePath } from '@shared/types/file' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -132,7 +133,7 @@ const FAKE_PDF_INFO = { modifiedAt: 1 } -const MARKDOWN_OUTPUT = { kind: 'path' as const, path: '/tmp/out.md' } +const MARKDOWN_OUTPUT = { kind: 'path' as const, path: '/tmp/out.md' as FilePath } const entryPayload = ( feature: 'image_to_text' | 'document_to_markdown', diff --git a/src/main/features/fileProcessing/persistence/artifacts.ts b/src/main/features/fileProcessing/persistence/artifacts.ts index d899dfe96df..f0d111ea784 100644 --- a/src/main/features/fileProcessing/persistence/artifacts.ts +++ b/src/main/features/fileProcessing/persistence/artifacts.ts @@ -26,7 +26,7 @@ export function getFileProcessingMarkdownArtifactPath(snapshot: JobSnapshot): Fi if (!isMarkdownFileArtifact(output.artifact)) { throw new Error(`File processing job ${snapshot.id} completed without a markdown path artifact`) } - return output.artifact.path as FilePath + return output.artifact.path } export function isMarkdownFileArtifact( @@ -73,7 +73,7 @@ async function createFileProcessingArtifact( path: await markdownResultStore.persistResultToPath({ jobId, result: output, - path: input.output.path as FilePath, + path: input.output.path, signal }) } diff --git a/src/main/features/fileProcessing/processors/__tests__/toPersistable.test.ts b/src/main/features/fileProcessing/processors/__tests__/toPersistable.test.ts index 77dfcbf3a66..79a24074307 100644 --- a/src/main/features/fileProcessing/processors/__tests__/toPersistable.test.ts +++ b/src/main/features/fileProcessing/processors/__tests__/toPersistable.test.ts @@ -16,8 +16,7 @@ import { mineruDocumentToMarkdownHandler } from '../mineru/document-to-markdown/ import { paddleDocumentToMarkdownHandler } from '../paddleocr/document-to-markdown/handler' import type { PreparedRemoteJob } from '../types' -const createFileInfo = (input: Parameters[0]): FileInfo => - FileInfoSchema.parse(input) as FileInfo +const createFileInfo = (input: Parameters[0]): FileInfo => FileInfoSchema.parse(input) const FAKE_PDF = createFileInfo({ path: '/tmp/paper.pdf', diff --git a/src/main/features/fileProcessing/processors/mistral/document-to-markdown/__tests__/handler.test.ts b/src/main/features/fileProcessing/processors/mistral/document-to-markdown/__tests__/handler.test.ts index ac99a4b7595..f80c596bbf4 100644 --- a/src/main/features/fileProcessing/processors/mistral/document-to-markdown/__tests__/handler.test.ts +++ b/src/main/features/fileProcessing/processors/mistral/document-to-markdown/__tests__/handler.test.ts @@ -194,5 +194,5 @@ function createFile(): FileInfo { type: 'document', createdAt: 1, modifiedAt: 1 - }) as FileInfo + }) } diff --git a/src/main/features/fileProcessing/processors/paddleocr/__tests__/handler.test.ts b/src/main/features/fileProcessing/processors/paddleocr/__tests__/handler.test.ts index e1791722bb6..ed8c5e0c634 100644 --- a/src/main/features/fileProcessing/processors/paddleocr/__tests__/handler.test.ts +++ b/src/main/features/fileProcessing/processors/paddleocr/__tests__/handler.test.ts @@ -1,7 +1,7 @@ import fs from 'node:fs/promises' import type { FileProcessorMerged } from '@shared/data/presets/fileProcessing' -import { type FileInfo, FileInfoSchema } from '@shared/types/file' +import { FileInfoSchema } from '@shared/types/file' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -53,7 +53,7 @@ const documentFile = FileInfoSchema.parse({ type: 'document', createdAt: 1, modifiedAt: 1 -}) as FileInfo +}) const imageFile = FileInfoSchema.parse({ path: '/tmp/input.png', @@ -64,7 +64,7 @@ const imageFile = FileInfoSchema.parse({ type: 'image', createdAt: 1, modifiedAt: 1 -}) as FileInfo +}) function createConfig(feature: 'image_to_text' | 'document_to_markdown', modelId: string): FileProcessorMerged { return { diff --git a/src/main/features/fileProcessing/processors/system/image-to-text/__tests__/handler.test.ts b/src/main/features/fileProcessing/processors/system/image-to-text/__tests__/handler.test.ts index a3e8049b18e..1d2a4401244 100644 --- a/src/main/features/fileProcessing/processors/system/image-to-text/__tests__/handler.test.ts +++ b/src/main/features/fileProcessing/processors/system/image-to-text/__tests__/handler.test.ts @@ -1,4 +1,4 @@ -import { FILE_TYPE, type FileInfo, FileInfoSchema } from '@shared/types/file' +import { FILE_TYPE, FileInfoSchema } from '@shared/types/file' import { beforeEach, describe, expect, it, vi } from 'vitest' import { mockMainLoggerService } from '../../../../../../../../tests/__mocks__/MainLoggerService' @@ -26,7 +26,7 @@ const imageFile = FileInfoSchema.parse({ type: FILE_TYPE.IMAGE, createdAt: 1, modifiedAt: 1 -}) as FileInfo +}) describe('systemImageToTextHandler', () => { beforeEach(() => { diff --git a/src/main/features/fileProcessing/processors/tesseract/image-to-text/__tests__/utils.test.ts b/src/main/features/fileProcessing/processors/tesseract/image-to-text/__tests__/utils.test.ts index c7268966960..960ebb335d4 100644 --- a/src/main/features/fileProcessing/processors/tesseract/image-to-text/__tests__/utils.test.ts +++ b/src/main/features/fileProcessing/processors/tesseract/image-to-text/__tests__/utils.test.ts @@ -1,4 +1,4 @@ -import { FILE_TYPE, type FileInfo, FileInfoSchema } from '@shared/types/file' +import { FILE_TYPE, FileInfoSchema } from '@shared/types/file' import { describe, expect, it, vi } from 'vitest' import { mockMainLoggerService } from '../../../../../../../../tests/__mocks__/MainLoggerService' @@ -13,7 +13,7 @@ const imageFile = FileInfoSchema.parse({ type: FILE_TYPE.IMAGE, createdAt: 1, modifiedAt: 1 -}) as FileInfo +}) describe('Tesseract prepareContext', () => { it('parses migrated langs arrays from processor options', () => { diff --git a/src/main/features/fileProcessing/processors/tesseract/runtime/__tests__/TesseractRuntimeService.test.ts b/src/main/features/fileProcessing/processors/tesseract/runtime/__tests__/TesseractRuntimeService.test.ts index 51b9b8a78d1..906e5c62590 100644 --- a/src/main/features/fileProcessing/processors/tesseract/runtime/__tests__/TesseractRuntimeService.test.ts +++ b/src/main/features/fileProcessing/processors/tesseract/runtime/__tests__/TesseractRuntimeService.test.ts @@ -6,6 +6,7 @@ import { getPhase } from '@main/core/lifecycle/decorators' import { Phase } from '@main/core/lifecycle/types' import { type FileInfo, FileInfoSchema } from '@shared/types/file' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as z from 'zod' import type { PreparedTesseractContext } from '../../types' @@ -48,7 +49,7 @@ async function flushPromises(): Promise { await Promise.resolve() } -function createFileInfo(overrides: Partial = {}): FileInfo { +function createFileInfo(overrides: Partial> = {}): FileInfo { return FileInfoSchema.parse({ path: '/tmp/scan.png', name: 'scan', @@ -59,7 +60,7 @@ function createFileInfo(overrides: Partial = {}): FileInfo { createdAt: 1, modifiedAt: 1, ...overrides - }) as FileInfo + }) } const cleanupCases = [ diff --git a/src/main/features/fileProcessing/tasks/__tests__/backgroundJobHandler.test.ts b/src/main/features/fileProcessing/tasks/__tests__/backgroundJobHandler.test.ts index 303220442e0..65dc990201a 100644 --- a/src/main/features/fileProcessing/tasks/__tests__/backgroundJobHandler.test.ts +++ b/src/main/features/fileProcessing/tasks/__tests__/backgroundJobHandler.test.ts @@ -7,6 +7,7 @@ * post-success output). */ import type { JobContext } from '@main/core/job/types' +import type { FilePath } from '@shared/types/file' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { FileProcessingJobPayload } from '../shared' @@ -174,7 +175,7 @@ describe('backgroundJobHandler.execute', () => { input: { feature: 'image_to_text', file: { kind: 'entry', entryId: FILE_ENTRY_ID }, - output: { kind: 'path', path: '/tmp/out.md' }, + output: { kind: 'path', path: '/tmp/out.md' as FilePath }, processorId: 'tesseract' } }) @@ -211,7 +212,7 @@ describe('backgroundJobHandler.execute', () => { input: { feature: 'image_to_text', file: { kind: 'entry', entryId: FILE_ENTRY_ID }, - output: { kind: 'path', path: '/tmp/out.md' }, + output: { kind: 'path', path: '/tmp/out.md' as FilePath }, processorId: 'tesseract' } }) diff --git a/src/main/features/fileProcessing/tasks/jobExecution.ts b/src/main/features/fileProcessing/tasks/jobExecution.ts index 9e8c6c95309..a21d95f3fa2 100644 --- a/src/main/features/fileProcessing/tasks/jobExecution.ts +++ b/src/main/features/fileProcessing/tasks/jobExecution.ts @@ -178,5 +178,5 @@ async function resolveFileProcessingPathInfo(filePath: FilePath): Promise { knowledgeItemUpdateStatusMock.mockReturnValueOnce(processingFile) knowledgeItemGetByIdMock.mockReturnValueOnce(processingFile) - await service.addItems('kb-1', [{ type: 'file', data: { source: '/docs/source.pdf', path: '/docs/source.pdf' } }]) + await service.addItems('kb-1', [ + { type: 'file', data: { source: '/docs/source.pdf', path: '/docs/source.pdf' as FilePath } } + ]) expect(fileProcessingStartJobMock).toHaveBeenCalledWith( { @@ -1017,8 +1020,8 @@ describe('KnowledgeService', () => { .mockReturnValueOnce(createFileItem('file-2', 'kb-1', '/Users/me/b/notes.md', 'processing')) await service.addItems('kb-1', [ - { type: 'file', data: { source: '/Users/me/a/notes.md', path: '/Users/me/a/notes.md' } }, - { type: 'file', data: { source: '/Users/me/b/notes.md', path: '/Users/me/b/notes.md' } } + { type: 'file', data: { source: '/Users/me/a/notes.md', path: '/Users/me/a/notes.md' as FilePath } }, + { type: 'file', data: { source: '/Users/me/b/notes.md', path: '/Users/me/b/notes.md' as FilePath } } ]) // Both imports land; the second's relativePath is deduped (`_N`) rather than refused. @@ -1041,8 +1044,8 @@ describe('KnowledgeService', () => { .mockReturnValueOnce(createFileItem('file-2', 'kb-1', '/Users/me/b/brief.docx', 'processing')) await service.addItems('kb-1', [ - { type: 'file', data: { source: '/Users/me/a/brief.pdf', path: '/Users/me/a/brief.pdf' } }, - { type: 'file', data: { source: '/Users/me/b/brief.docx', path: '/Users/me/b/brief.docx' } } + { type: 'file', data: { source: '/Users/me/a/brief.pdf', path: '/Users/me/a/brief.pdf' as FilePath } }, + { type: 'file', data: { source: '/Users/me/b/brief.docx', path: '/Users/me/b/brief.docx' as FilePath } } ]) // brief.pdf reserves brief.pdf + its brief.md output; brief.docx would also emit @@ -1070,7 +1073,7 @@ describe('KnowledgeService', () => { data: { source: 'https://example.com/new', url: 'https://example.com/new', - snapshotPath: '/captured/example-page.md' + snapshotPath: '/captured/example-page.md' as FilePath } } ]) @@ -1108,7 +1111,7 @@ describe('KnowledgeService', () => { data: { source: 'https://example.com/p', url: 'https://example.com/p', - snapshotPath: '/captured/example-page.md' + snapshotPath: '/captured/example-page.md' as FilePath } } ]) @@ -1144,7 +1147,7 @@ describe('KnowledgeService', () => { ) await service.addItems('kb-1', [ - { type: 'file', data: { source: '/Users/me/Meeting notes.md', path: '/Users/me/Meeting notes.md' } } + { type: 'file', data: { source: '/Users/me/Meeting notes.md', path: '/Users/me/Meeting notes.md' as FilePath } } ]) // The new file's name collides with the existing note's reserved snapshot path, so it is @@ -1203,7 +1206,7 @@ describe('KnowledgeService', () => { knowledgeItemGetItemsByBaseIdMock.mockReturnValue([createFileItem('file-existing', 'kb-1', '/old/notes.md')]) await service.addItems('kb-1', [ - { type: 'file', data: { source: '/Users/me/c/notes.md', path: '/Users/me/c/notes.md' } } + { type: 'file', data: { source: '/Users/me/c/notes.md', path: '/Users/me/c/notes.md' as FilePath } } ]) expect(copyFileIntoKnowledgeBaseAtMock).toHaveBeenCalledWith('kb-1', '/Users/me/c/notes.md', 'notes_1.md') @@ -1218,7 +1221,7 @@ describe('KnowledgeService', () => { knowledgeItemGetItemsByBaseIdMock.mockReturnValue([createFileItem('file-existing', 'kb-1', '/old/brief.pdf')]) await service.addItems('kb-1', [ - { type: 'file', data: { source: '/Users/me/c/brief.md', path: '/Users/me/c/brief.md' } } + { type: 'file', data: { source: '/Users/me/c/brief.md', path: '/Users/me/c/brief.md' as FilePath } } ]) expect(copyFileIntoKnowledgeBaseAtMock).toHaveBeenCalledWith('kb-1', '/Users/me/c/brief.md', 'brief_1.md') @@ -1229,7 +1232,9 @@ describe('KnowledgeService', () => { knowledgeBaseGetByIdMock.mockReturnValue(createBase({ fileProcessorId: null })) await expect( - service.addItems('kb-1', [{ type: 'file', data: { source: '/Users/me/app.exe', path: '/Users/me/app.exe' } }]) + service.addItems('kb-1', [ + { type: 'file', data: { source: '/Users/me/app.exe', path: '/Users/me/app.exe' as FilePath } } + ]) ).rejects.toThrow('Unsupported knowledge file type: /Users/me/app.exe') expect(knowledgeItemCreateMock).not.toHaveBeenCalled() @@ -1405,7 +1410,9 @@ describe('KnowledgeService', () => { knowledgeItemUpdateStatusMock.mockReturnValueOnce(processingFile) knowledgeItemGetByIdMock.mockReturnValueOnce(processingFile) - await service.addItems('kb-1', [{ type: 'file', data: { source: '/docs/source.md', path: '/docs/source.md' } }]) + await service.addItems('kb-1', [ + { type: 'file', data: { source: '/docs/source.md', path: '/docs/source.md' as FilePath } } + ]) expect(fileProcessingStartJobMock).not.toHaveBeenCalled() expect(enqueueMock).toHaveBeenCalledWith( @@ -1428,7 +1435,9 @@ describe('KnowledgeService', () => { knowledgeItemUpdateStatusMock.mockReturnValueOnce(processingFile) knowledgeItemGetByIdMock.mockReturnValueOnce(processingFile) - await service.addItems('kb-1', [{ type: 'file', data: { source: '/docs/source.pdf', path: '/docs/source.pdf' } }]) + await service.addItems('kb-1', [ + { type: 'file', data: { source: '/docs/source.pdf', path: '/docs/source.pdf' as FilePath } } + ]) expect(fileProcessingStartJobMock).not.toHaveBeenCalled() expect(enqueueMock).toHaveBeenCalledWith( @@ -1492,7 +1501,7 @@ describe('KnowledgeService', () => { }) await expect( - service.addItems('kb-1', [{ type: 'file', data: { source: '/docs/x.pdf', path: '/docs/x.pdf' } }]) + service.addItems('kb-1', [{ type: 'file', data: { source: '/docs/x.pdf', path: '/docs/x.pdf' as FilePath } }]) ).rejects.toThrow('create failed') // Copied-file cleanup is delegated to the best-effort variant, which swallows its diff --git a/src/main/features/knowledge/utils/__tests__/addConflicts.test.ts b/src/main/features/knowledge/utils/__tests__/addConflicts.test.ts index 5b18a8fe0a3..70c405ad9dc 100644 --- a/src/main/features/knowledge/utils/__tests__/addConflicts.test.ts +++ b/src/main/features/knowledge/utils/__tests__/addConflicts.test.ts @@ -1,4 +1,5 @@ import type { KnowledgeAddItemInput, KnowledgeItem } from '@shared/data/types/knowledge' +import type { FilePath } from '@shared/types/file' import { describe, expect, it } from 'vitest' import { resolveKnowledgeAddConflicts } from '../addConflicts' @@ -15,7 +16,10 @@ const existingItem = (id: string, partial: Pick) ...partial }) as KnowledgeItem -const fileInput = (source: string): KnowledgeAddItemInput => ({ type: 'file', data: { source, path: source } }) +const fileInput = (source: string): KnowledgeAddItemInput => ({ + type: 'file', + data: { source, path: source as FilePath } +}) const urlInput = (url: string): KnowledgeAddItemInput => ({ type: 'url', data: { source: url, url } }) const noteInput = (content: string): KnowledgeAddItemInput => ({ type: 'note', data: { source: 'note', content } }) diff --git a/src/main/features/knowledge/utils/storage/pathStorage.ts b/src/main/features/knowledge/utils/storage/pathStorage.ts index 820e1131e84..30b0259e3e2 100644 --- a/src/main/features/knowledge/utils/storage/pathStorage.ts +++ b/src/main/features/knowledge/utils/storage/pathStorage.ts @@ -6,7 +6,7 @@ import { loggerService } from '@logger' import { copy, ensureDir, type PathReadability, probeReadable, remove, removeDir, write } from '@main/utils/file' import { nextFreeKnowledgeRelativePath } from '@main/utils/knowledge' import { getFileExt } from '@main/utils/legacyFile' -import type { FilePath } from '@shared/types/file' +import { type FilePath, FilePathSchema } from '@shared/types/file' import { knowledgeFileProcessingExts } from '@shared/utils/file' const logger = loggerService.withContext('Knowledge:PathStorage') @@ -31,16 +31,16 @@ const VECTOR_STORE_FILE = 'index.sqlite' const MATERIAL_ROOT_DIR = 'raw' export function getKnowledgeBaseDir(baseId: string): FilePath { - return path.join(application.getPath('feature.knowledgebase.data'), baseId) as FilePath + return FilePathSchema.parse(path.join(application.getPath('feature.knowledgebase.data'), baseId)) } /** The material root (`{baseDir}/raw`) under which every `relativePath` resolves. */ export function getKnowledgeMaterialDir(baseId: string): FilePath { - return path.join(getKnowledgeBaseDir(baseId), MATERIAL_ROOT_DIR) as FilePath + return FilePathSchema.parse(path.join(getKnowledgeBaseDir(baseId), MATERIAL_ROOT_DIR)) } export function getKnowledgeBaseMetaDir(baseId: string): FilePath { - return path.join(getKnowledgeBaseDir(baseId), CHERRY_META_DIR) as FilePath + return FilePathSchema.parse(path.join(getKnowledgeBaseDir(baseId), CHERRY_META_DIR)) } /** @@ -51,12 +51,12 @@ export function getKnowledgeBaseMetaDir(baseId: string): FilePath { */ export function getKnowledgeVectorStoreFilePathSync(baseId: string): FilePath { const metaDir = getKnowledgeBaseMetaDir(baseId) - return path.join(metaDir, VECTOR_STORE_FILE) as FilePath + return FilePathSchema.parse(path.join(metaDir, VECTOR_STORE_FILE)) } export function getKnowledgeBaseFilePath(baseId: string, relativePath: string): FilePath { assertSafeKnowledgeRelativePath(relativePath) - return path.join(getKnowledgeMaterialDir(baseId), relativePath) as FilePath + return FilePathSchema.parse(path.join(getKnowledgeMaterialDir(baseId), relativePath)) } /** @@ -75,7 +75,7 @@ export async function probeKnowledgeFile(baseId: string, relativePath: string): * is already absolute, so it is probed as-is. */ export async function probeKnowledgeSourcePath(absolutePath: string): Promise { - return probeReadable(absolutePath as FilePath) + return probeReadable(FilePathSchema.parse(absolutePath)) } export function getKnowledgeSourceRelativePath(sourcePath: string): string { @@ -135,8 +135,8 @@ export async function copyFileIntoKnowledgeBaseAt( ): Promise { const destPath = getKnowledgeBaseFilePath(baseId, relativePath) await assertTargetAvailable(destPath) - await ensureDir(path.dirname(destPath) as FilePath) - await copy(sourcePath as FilePath, destPath) + await ensureDir(FilePathSchema.parse(path.dirname(destPath))) + await copy(FilePathSchema.parse(sourcePath), destPath) return relativePath } @@ -148,7 +148,7 @@ export async function writeFileIntoKnowledgeBaseAt( ): Promise { const destPath = getKnowledgeBaseFilePath(baseId, relativePath) await assertTargetAvailable(destPath) - await ensureDir(path.dirname(destPath) as FilePath) + await ensureDir(FilePathSchema.parse(path.dirname(destPath))) await write(destPath, content) return relativePath } diff --git a/src/main/ipc/handlers/__tests__/file.test.ts b/src/main/ipc/handlers/__tests__/file.test.ts index dfec31c318c..998d15048f3 100644 --- a/src/main/ipc/handlers/__tests__/file.test.ts +++ b/src/main/ipc/handlers/__tests__/file.test.ts @@ -22,6 +22,8 @@ vi.mock('@main/services/file', async () => { } }) +import type { FilePath } from '@shared/types/file' + import { fileHandlers } from '../file' const ids = ['019606a0-0000-7000-8000-000000000001', '019606a0-0000-7000-8000-000000000002'] @@ -65,7 +67,7 @@ describe('fileHandlers', () => { it('batch_get_metadata dispatches FileHandle items inside the IPC adapter', async () => { const items = [ { key: ids[0], handle: { kind: 'entry' as const, entryId: ids[0] } }, - { key: '/tmp/a.txt', handle: { kind: 'path' as const, path: '/tmp/a.txt' } }, + { key: '/tmp/a.txt', handle: { kind: 'path' as const, path: '/tmp/a.txt' as FilePath } }, { key: ids[1], handle: { kind: 'entry' as const, entryId: ids[1] } } ] fileManager.getMetadata.mockResolvedValueOnce(metadata).mockRejectedValueOnce(new Error('ENOENT')) @@ -130,8 +132,8 @@ describe('fileHandlers', () => { }) it('dispatches path system commands without FileManager entry lookup', async () => { - await fileHandlers['file.open']({ kind: 'path', path: '/tmp/report.md' }, ctx) - await fileHandlers['file.show_in_folder']({ kind: 'path', path: '/tmp/report.md' }, ctx) + await fileHandlers['file.open']({ kind: 'path', path: '/tmp/report.md' as FilePath }, ctx) + await fileHandlers['file.show_in_folder']({ kind: 'path', path: '/tmp/report.md' as FilePath }, ctx) expect(safeOpenMock).toHaveBeenCalledWith('/tmp/report.md') expect(showPathInFolderMock).toHaveBeenCalledWith('/tmp/report.md') @@ -142,8 +144,8 @@ describe('fileHandlers', () => { it('delegates internal-entry batch create items to FileManager', async () => { const result = { succeeded: [{ id: ids[0], sourceRef: '/tmp/a.txt' }], failed: [] } const items = [ - { source: 'path' as const, path: '/tmp/a.txt' }, - { source: 'path' as const, path: '/tmp/b.txt' } + { source: 'path' as const, path: '/tmp/a.txt' as FilePath }, + { source: 'path' as const, path: '/tmp/b.txt' as FilePath } ] fileManager.batchCreateInternalEntries.mockResolvedValue(result) diff --git a/src/main/services/file/FileManager.ts b/src/main/services/file/FileManager.ts index bd478bdd73f..9fffee8283c 100644 --- a/src/main/services/file/FileManager.ts +++ b/src/main/services/file/FileManager.ts @@ -134,7 +134,7 @@ import { loggerService } from '@logger' import { BaseService, Injectable, Phase, ServicePhase } from '@main/core/lifecycle' import { remove as fsRemove, stat as fsStat } from '@main/utils/file' import type { DanglingState, FileEntry, FileEntryId, FileHandle } from '@shared/data/types/file' -import { AbsolutePathSchema, FileEntryIdSchema, FileHandleSchema, SafeNameSchema } from '@shared/data/types/file' +import { FileEntryIdSchema, FileHandleSchema, SafeNameSchema } from '@shared/data/types/file' import { IpcChannel } from '@shared/IpcChannel' import type { BatchCreateResult, @@ -145,7 +145,8 @@ import type { FileUrlString, PhysicalFileMetadata } from '@shared/types/file' -import { SafeExtSchema } from '@shared/types/file' +import { FilePathSchema, SafeExtSchema } from '@shared/types/file' +import { canonicalizeFilePath } from '@shared/utils/file' import mime from 'mime' import * as z from 'zod' @@ -186,7 +187,7 @@ import { showInFolder as internalShellShowInFolder } from './internal/system/she import { withTempCopy as internalWithTempCopy } from './internal/system/tempCopy' import { safeOpen } from './system' import { getMetadataByPath } from './utils/metadata' -import { canonicalizeExternalPath, resolvePhysicalPath } from './utils/pathResolver' +import { resolvePhysicalPath } from './utils/pathResolver' import { createVersionCacheImpl, type VersionCache } from './versionCache' const fileManagerLogger = loggerService.withContext('FileManager') @@ -229,7 +230,7 @@ export type EnsureExternalEntryParams = EnsureExternalEntryIpcParams const SafeExtNullableSchema = SafeExtSchema.nullable() export const CreateInternalEntryIpcSchema = z.discriminatedUnion('source', [ - z.strictObject({ source: z.literal('path'), path: AbsolutePathSchema }), + z.strictObject({ source: z.literal('path'), path: FilePathSchema }), z.strictObject({ source: z.literal('url'), url: z.url() }), z.strictObject({ source: z.literal('base64'), data: z.string().min(1), name: SafeNameSchema.optional() }), z.strictObject({ @@ -240,7 +241,7 @@ export const CreateInternalEntryIpcSchema = z.discriminatedUnion('source', [ }) ]) -export const EnsureExternalEntryIpcSchema = z.strictObject({ externalPath: AbsolutePathSchema }) +export const EnsureExternalEntryIpcSchema = z.strictObject({ externalPath: FilePathSchema }) export const GetPhysicalPathIpcSchema = z.strictObject({ id: FileEntryIdSchema }) @@ -820,8 +821,8 @@ export class FileManager extends BaseService implements IFileManager { return this.deps.fileEntryService.findById(id) } - async findByExternalPath(rawPath: string): Promise { - return this.deps.fileEntryService.findByExternalPath(canonicalizeExternalPath(rawPath)) + async findByExternalPath(path: FilePath): Promise { + return this.deps.fileEntryService.findByExternalPath(canonicalizeFilePath(path)) } async ensureExternalEntry(params: EnsureExternalEntryParams): Promise { @@ -911,19 +912,22 @@ export class FileManager extends BaseService implements IFileManager { async batchEnsureExternalEntries(items: EnsureExternalEntryParams[]): Promise { // Within-batch path duplicates resolve to the same entry per the public // contract; the second occurrence reuses the just-inserted row. The - // canonical-path memoization here ensures both items end up in - // `succeeded` even though only one DB insert happens — and each carries - // its own `sourceRef`, so the caller can still correlate every input. + // in-memory memo keys on the branded `externalPath` directly — no re-parse, + // trusting the already-validated `FilePath` param. Byte-identical inputs + // dedup here; any canonically-equal-but-byte-different pair still coalesces + // one level down (`ensureExternalEntry` canonicalizes and hits the DB + // upsert). Both items end up in `succeeded` even though only one DB insert + // happens — and each carries its own `sourceRef`, so the caller can still + // correlate every input. const seen = new Map() const succeeded: BatchCreateResult['succeeded'] = [] const failed: BatchCreateResult['failed'] = [] for (const params of items) { const sourceRef = params.externalPath try { - const canonical = canonicalizeExternalPath(params.externalPath) - const cached = seen.get(canonical) + const cached = seen.get(params.externalPath) const entry = cached ?? (await this.ensureExternalEntry(params)) - if (!cached) seen.set(canonical, entry) + if (!cached) seen.set(params.externalPath, entry) succeeded.push({ id: entry.id, sourceRef }) } catch (err) { // Wire format only carries `.message`; preserve the stack via the diff --git a/src/main/services/file/__tests__/FileManager.integration.test.ts b/src/main/services/file/__tests__/FileManager.integration.test.ts index a5e6e419ef3..70ff38b9a4c 100644 --- a/src/main/services/file/__tests__/FileManager.integration.test.ts +++ b/src/main/services/file/__tests__/FileManager.integration.test.ts @@ -5,8 +5,9 @@ import path from 'node:path' import { application } from '@application' import { fileEntryTable } from '@data/db/schemas/file' import { BaseService } from '@main/core/lifecycle' -import type { FileEntryId } from '@shared/data/types/file' +import { type FileEntryId } from '@shared/data/types/file' import { fileErrorCodes } from '@shared/ipc/errors/file' +import { FilePathSchema } from '@shared/types/file' import { setupTestDatabase } from '@test-helpers/db' import { MockMainCacheServiceUtils } from '@test-mocks/main/CacheService' import { MockMainDbServiceUtils } from '@test-mocks/main/DbService' @@ -122,12 +123,14 @@ describe('FileManager (integration)', () => { }) // Canonical lookup - const found = await fm.findByExternalPath(`${file}/`) // trailing slash → canonicalize strips + const found = await fm.findByExternalPath(FilePathSchema.parse(`${file}/`)) // trailing slash → canonicalize strips expect(found?.id).toBe(id) - // NFC re-normalization survives a synthesized NFD form + // Byte-faithful lookup: canonicalization does NOT Unicode-normalize, so an + // NFD synthesis of this ASCII path is byte-identical and matches the stored + // (byte-faithful) row exactly. const nfdFile = file.normalize('NFD') - const foundNfc = await fm.findByExternalPath(nfdFile) + const foundNfc = await fm.findByExternalPath(FilePathSchema.parse(nfdFile)) expect(foundNfc?.id).toBe(id) // Content hash works for external entries @@ -541,29 +544,50 @@ describe('FileManager (integration)', () => { expect(rows).toHaveLength(2) }) - it('INT-15b: batchEnsureExternalEntries dedupes within-batch duplicate paths and aggregates per-item failures', async () => { + it('INT-15b: batchEnsureExternalEntries dedupes duplicates, creates a dangling entry for a missing path, and aggregates a genuine per-item failure', async () => { const same = path.join(tmp, 'dedupe.txt') await writeFile(same, 'x') - const missing = path.join(tmp, 'no-such-file.txt') + const missing = path.join(tmp, 'no-such-file.txt') // absent on disk → dangling success + const guarded = path.join(tmp, 'guarded.txt') // stat EACCES → genuine per-item failure + + // A missing external path is a first-class dangling state, not an error, so + // only a *genuine* FS fault still fails an item. Drive the fs stat seam to + // throw EACCES for `guarded` alone; every other path stats for real. + const fsModule = await import('@main/utils/file/fs') + const realStat = fsModule.stat + const statSpy = vi.spyOn(fsModule, 'stat').mockImplementation(async (p) => { + if (p === guarded) throw Object.assign(new Error('permission denied'), { code: 'EACCES' }) + return realStat(p) + }) const result = await fm.batchEnsureExternalEntries([ { externalPath: same as never }, { externalPath: same as never }, - { externalPath: missing as never } + { externalPath: missing as never }, + { externalPath: guarded as never } ]) // Two `same`-path inputs collapse to ONE DB row, but BOTH appear in // succeeded with the matching sourceRef so callers can still correlate // each input — that is the dedupe contract the BatchCreateResult split - // (I3) was designed to express. - expect(result.succeeded).toHaveLength(2) + // (I3) was designed to express. The missing path succeeds as a DANGLING + // entry; only the EACCES fault lands in failed. + expect(result.succeeded).toHaveLength(3) expect(result.succeeded[0].sourceRef).toBe(same) expect(result.succeeded[1].sourceRef).toBe(same) expect(result.succeeded[0].id).toBe(result.succeeded[1].id) + expect(result.succeeded[2].sourceRef).toBe(missing) expect(result.failed).toHaveLength(1) - expect(result.failed[0].sourceRef).toBe(missing) - // The DB must contain exactly one external row for `same`. + expect(result.failed[0].sourceRef).toBe(guarded) + // The missing path produced a real external row whose dangling state is 'missing'. + const missingEntry = await fm.findByExternalPath(FilePathSchema.parse(missing)) + if (!missingEntry) throw new Error('expected a dangling entry for the missing path') + expect(await fm.getDanglingState({ id: missingEntry.id })).toBe('missing') + // Exactly one external row for `same`; the EACCES fault left no row. const rows = await dbh.db.select().from(fileEntryTable) expect(rows.filter((r) => r.externalPath === same)).toHaveLength(1) + expect(rows.filter((r) => r.externalPath === guarded)).toHaveLength(0) + + statSpy.mockRestore() }) it('INT-8: batchGetDanglingStates returns "unknown" for ids that have no entry', async () => { diff --git a/src/main/services/file/__tests__/watcher.test.ts b/src/main/services/file/__tests__/watcher.test.ts index 9b0d10f2389..7725d6ed36e 100644 --- a/src/main/services/file/__tests__/watcher.test.ts +++ b/src/main/services/file/__tests__/watcher.test.ts @@ -182,28 +182,27 @@ describe('createDirectoryWatcher', () => { await w.close() }) - // Reverse-direction skip from the NFD test in entry/create.test.ts: - // chokidar emits whatever the OS hands it. On macOS APFS / Windows NTFS the - // FS normalizes to NFC at storage time, so even a file we name with NFD - // bytes gets surfaced as NFC and the case under test (chokidar firing NFD) - // can't be set up locally. On Linux ext4, filenames are opaque bytes — - // writeFile preserves the NFD encoding verbatim, chokidar surfaces it as - // NFD, which is exactly the scenario the production fix targets (a CJK - // file migrated from HFS+ via `rsync -E` arrives in NFD on macOS users' - // disks; we use Linux to *reproduce* that byte pattern under test). + // On Linux ext4, filenames are opaque bytes — writeFile preserves the NFD + // encoding verbatim and chokidar surfaces it as NFD, so we use Linux to + // *reproduce* the byte pattern a CJK/accented file migrated from HFS+ (via + // `rsync -E`) shows up as on a macOS user's disk. Because `externalPath` is + // now stored byte-faithful (no NFC), the DanglingCache reverse index is + // keyed by that exact NFD form and the watcher matches the chokidar event by + // *raw byte equality* — the previous NFC-normalize bridge in `handle()` is + // gone. On Linux the raw event byte-matches the stored key by construction. it.runIf(process.platform === 'linux')( - 'normalizes NFD chokidar paths to NFC before feeding DanglingCache (linux-reproducible regression)', + 'feeds DanglingCache by raw byte equality — NFD event matches the byte-faithful NFD key (no NFC step)', async () => { const nfd = 'qu\u0065\u0301.txt' // q, u, e, combining acute -> NFD const nfc = 'qu\u00E9.txt' // q, u, e-precomposed -> NFC expect(nfd).not.toBe(nfc) // byte-distinct strings reaching us at runtime const writtenPath = path.join(dir, nfd) as FilePath - const canonicalPath = path.join(dir, nfc) as FilePath - // DanglingCache's reverse index is populated by `ensureExternalEntry` → - // `canonicalizeExternalPath` which already lands NFC. Mirror that here. - danglingCache.addEntry('e-w-nfd' as FileEntryId, canonicalPath) + // DanglingCache's reverse index is populated by `ensureExternalEntry`, + // whose `externalPath` is now stored byte-faithful (no NFC). Mirror that + // by registering the entry under the exact NFD bytes on disk. + danglingCache.addEntry('e-w-nfd' as FileEntryId, writtenPath) const w = createDirectoryWatcher(dir as FilePath) await waitForReady(w) @@ -212,13 +211,13 @@ describe('createDirectoryWatcher', () => { if (ev.kind !== 'add') throw new Error('expected add event') expect(ev.path).toBe(writtenPath) - // The cache lookup uses NFC keys; without the NFC-normalize step in - // `handle()` this would miss and the cache would stay 'unknown'. + // The reverse-index key and the chokidar event path are the same + // byte-faithful NFD string, so the lookup hits with no NFC normalization. expect( await danglingCache.check({ id: 'e-w-nfd' as FileEntryId, origin: 'external', - externalPath: canonicalPath, + externalPath: writtenPath, name: 'qué', ext: 'txt', size: null, diff --git a/src/main/services/file/internal/__tests__/observe.test.ts b/src/main/services/file/internal/__tests__/observe.test.ts index b64490a6049..db406272674 100644 --- a/src/main/services/file/internal/__tests__/observe.test.ts +++ b/src/main/services/file/internal/__tests__/observe.test.ts @@ -17,10 +17,10 @@ import type { FileManagerDeps } from '../deps' import { observeExternalAccess } from '../observe' // `as unknown as FileEntry` because `externalPath` is now branded as -// `CanonicalFilePath` (FilePath & CanonicalExternalPath) — a string literal -// can't satisfy the brand directly. The actual canonicalization invariant -// is irrelevant for these tests (we never feed the entry back into the -// schema); they only need the discriminator + a stable physical path. +// `FilePath` (via `FilePathSchema`) — a string literal can't satisfy the +// brand directly. The actual canonicalization invariant is irrelevant for +// these tests (we never feed the entry back into the schema); they only +// need the discriminator + a stable physical path. const externalEntry: FileEntry = { id: '019606a0-0000-7000-8000-0000000000ee' as FileEntryId, origin: 'external', diff --git a/src/main/services/file/internal/content/__tests__/read.test.ts b/src/main/services/file/internal/content/__tests__/read.test.ts index 22232282156..50b2ed51693 100644 --- a/src/main/services/file/internal/content/__tests__/read.test.ts +++ b/src/main/services/file/internal/content/__tests__/read.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { fileEntryTable } from '@data/db/schemas/file' -import type { CanonicalExternalPath, FileEntryId } from '@shared/data/types/file' +import type { FileEntryId } from '@shared/data/types/file' import type { FilePath } from '@shared/types/file' import { setupTestDatabase } from '@test-helpers/db' import { MockMainDbServiceUtils } from '@test-mocks/main/DbService' @@ -157,12 +157,4 @@ describe('internal/content/read', () => { const result = await readByPath(deps, file) expect(result.content).toBe('direct content') }) - - it('proves CanonicalExternalPath brand is unused (read uses raw FilePath)', () => { - // Sanity: external-path lookup goes through fileEntryService.findByExternalPath, - // not through internal/content/read. This test exists to prevent accidental - // signature drift between modules. - const _brand: CanonicalExternalPath = '/tmp/x' as CanonicalExternalPath - expect(typeof _brand).toBe('string') - }) }) diff --git a/src/main/services/file/internal/entry/__tests__/create.test.ts b/src/main/services/file/internal/entry/__tests__/create.test.ts index 2a11f1e3e0f..1fafcb37d90 100644 --- a/src/main/services/file/internal/entry/__tests__/create.test.ts +++ b/src/main/services/file/internal/entry/__tests__/create.test.ts @@ -3,7 +3,7 @@ import type { Server } from 'node:http' import { tmpdir } from 'node:os' import path from 'node:path' -import type { FilePath } from '@shared/types/file' +import { type FilePath, FilePathSchema } from '@shared/types/file' import { setupTestDatabase } from '@test-helpers/db' import { MockMainDbServiceUtils } from '@test-mocks/main/DbService' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -233,6 +233,43 @@ describe('internal/entry/create.createInternal', () => { }) }) + describe('ensureExternal dangling create (missing path is a first-class state, not an error)', () => { + it('creates an external entry for a path that does not exist and seeds a "missing" observation', async () => { + // The file is never written to disk. `ensureExternal` probes presence + // best-effort: ENOENT means "simply absent" → create the entry DANGLING + // rather than throw (an external ref to an off-disk path is a first-class + // state). The returned row is a real external entry, and the DanglingCache + // observation seeded on insert is 'missing' — i.e. a subsequent + // `danglingCache.check(entry)` resolves to 'missing'. + const missing = path.join(tmp, 'never-written.pdf') + const entry = await ensureExternal(deps, { externalPath: missing as FilePath }) + if (entry.origin !== 'external') throw new Error('expected external entry') + expect(entry.externalPath).toBe(missing) + expect(entry.name).toBe('never-written') + expect(entry.ext).toBe('pdf') + // Persisted, not just returned. + expect(fileEntryService.getById(entry.id).id).toBe(entry.id) + // Reverse-indexed and seeded 'missing' (dangling), NOT 'present'. + expect(deps.danglingCache.addEntry).toHaveBeenCalledWith(entry.id, entry.externalPath) + expect(deps.danglingCache.onFsEvent).toHaveBeenCalledWith(entry.externalPath, 'missing', 'ops') + }) + + it('rethrows a non-ENOENT stat error (EACCES) instead of creating a dangling entry', async () => { + // ENOENT / ENOTDIR flip to dangling-create; every other errno is a + // genuine FS fault that must preserve the old fail-loud behavior. Drive + // the same `@main/utils/file/fs` stat seam that write/rename suites mock, + // making it throw an EACCES-coded error. + const fsModule = await import('@main/utils/file/fs') + const guarded = path.join(tmp, 'guarded.txt') + const statErr = Object.assign(new Error('permission denied'), { code: 'EACCES' }) + vi.spyOn(fsModule, 'stat').mockRejectedValue(statErr) + await expect(ensureExternal(deps, { externalPath: guarded as FilePath })).rejects.toBe(statErr) + // No entry inserted, no cache observation recorded on the fail-loud path. + expect(deps.danglingCache.addEntry).not.toHaveBeenCalled() + expect(deps.danglingCache.onFsEvent).not.toHaveBeenCalled() + }) + }) + describe('ensureExternal case-collision policy (M2: functional unique index + fs.realpath)', () => { // Background: `fe_external_path_lower_unique_idx` enforces case-insensitive // uniqueness on `externalPath` at the DB layer. Application-side, the @@ -305,41 +342,36 @@ describe('internal/entry/create.createInternal', () => { ) }) - describe('ensureExternal canonical derivation', () => { - // Skip on linux: ext4 stores filenames as opaque bytes (no NFC/NFD - // equivalence), so a file written under an NFD name is genuinely a - // different FS entry from the NFC form — statting the canonical ENOENTs - // at the FS layer before the derivation invariant is exercised. The bug - // this guards is an APFS / NTFS concern (silent NFC-vs-NFD divergence - // between raw drag-drop input and `canonical`), which the macOS / Windows - // runners do exercise. - it.skipIf(process.platform === 'linux')( - 'derives name/ext from the canonical path, not the raw input (NFD → NFC byte equivalence)', - async () => { - // Regression guard: previously `name = params.name ?? defaultNameFromPath(params.externalPath)` - // and `ext = extWithoutDot(params.externalPath)` derived from the raw - // input. On macOS APFS the raw input can arrive in NFD form while - // `canonical` is NFC — persisting NFD-encoded name/ext alongside an - // NFC externalPath silently breaks `path.basename(canonical) === entry.name` - // equality checks. The fix derives every field from `canonical`. - const nfdName = 'qué' // 'qué' = e + combining acute (NFD) - const nfcName = 'qué' // 'qué' = single codepoint (NFC) - expect(nfdName).not.toBe(nfcName) // byte-distinct strings - expect(nfdName.normalize('NFC')).toBe(nfcName) - - const file = path.join(tmp, `${nfdName}.txt`) - await writeFile(file, 'x') - const entry = await ensureExternal(deps, { externalPath: file as FilePath }) - - if (entry.origin !== 'external') throw new Error('expected external entry') - // The stored externalPath is NFC (canonicalize applies .normalize('NFC')). - const canonical = entry.externalPath - expect(canonical.normalize('NFC')).toBe(canonical) - // name must derive from the canonical (NFC) basename, not the raw NFD input. - expect(entry.name).toBe(nfcName) - // Round-trip equality through path.basename now holds. - expect(path.basename(canonical, '.txt')).toBe(entry.name) - } - ) + describe('ensureExternal byte-faithful derivation', () => { + // `externalPath` is stored byte-faithful — `canonicalizeFilePath` does NOT + // Unicode-normalize, so an NFD-named file keeps its NFD bytes end to end: + // the stored path reaches the real file on every filesystem (including + // Linux ext4, where an NFC-rewritten path would ENOENT), and `name`/`ext` + // are derived from that byte-faithful path, not folded to NFC. Runs on all + // platforms: the byte-faithful path matches the on-disk bytes everywhere. + it('derives name/ext from the byte-faithful canonical path (NFD stays NFD, no NFC fold)', async () => { + // ASCII \u escapes (not raw accented literals) so formatter/editor tooling + // cannot silently re-normalize the NFD form and turn this into a tautology. + const nfdName = 'qu\u0065\u0301' // q, u, e, combining acute -> NFD + const nfcName = 'qu\u00E9' // q, u, e-precomposed -> NFC + expect(nfdName).not.toBe(nfcName) // byte-distinct strings + expect(nfdName.normalize('NFC')).toBe(nfcName) + + const file = path.join(tmp, `${nfdName}.txt`) + await writeFile(file, 'x') + const entry = await ensureExternal(deps, { externalPath: FilePathSchema.parse(file) }) + + if (entry.origin !== 'external') throw new Error('expected external entry') + // The stored externalPath is byte-faithful — the exact NFD bytes we passed, + // NOT folded to NFC. + const canonical = entry.externalPath + expect(canonical).toBe(file) + expect(canonical).not.toBe(file.normalize('NFC')) + // name derives from the byte-faithful (NFD) basename, not an NFC fold. + expect(entry.name).toBe(nfdName) + expect(entry.name).not.toBe(nfcName) + // Round-trip equality through path.basename holds byte-for-byte. + expect(path.basename(canonical, '.txt')).toBe(entry.name) + }) }) }) diff --git a/src/main/services/file/internal/entry/__tests__/lifecycle.test.ts b/src/main/services/file/internal/entry/__tests__/lifecycle.test.ts index fdf637b178c..eb474ab48b3 100644 --- a/src/main/services/file/internal/entry/__tests__/lifecycle.test.ts +++ b/src/main/services/file/internal/entry/__tests__/lifecycle.test.ts @@ -135,10 +135,10 @@ describe('internal/entry/lifecycle', () => { const entry = fileEntryService.getById(id) if (entry.origin !== 'external') throw new Error('expected external entry') const userFile = entry.externalPath - expect(await exists(userFile as FilePath)).toBe(true) + expect(await exists(userFile)).toBe(true) await permanentDelete(deps, id) expect(fileEntryService.findById(id)).toBeNull() - expect(await exists(userFile as FilePath)).toBe(true) + expect(await exists(userFile)).toBe(true) }) it('still deletes the row when the internal physical file is missing', async () => { diff --git a/src/main/services/file/internal/entry/__tests__/rename.test.ts b/src/main/services/file/internal/entry/__tests__/rename.test.ts index 2f70d7dbe96..13fca15d5fe 100644 --- a/src/main/services/file/internal/entry/__tests__/rename.test.ts +++ b/src/main/services/file/internal/entry/__tests__/rename.test.ts @@ -119,21 +119,22 @@ describe('internal/entry/rename', () => { expect(await readFile(collision, 'utf-8')).toBe('B') }) - it('treats NFC/NFD-equivalent names as a no-op (no fs.rename, no DB write)', async () => { - // macOS HFS+/APFS surface filenames in NFD; renderer input is NFC. - // path.join produces a string whose codepoints differ from the stored - // (NFC) externalPath even though they refer to the same logical file. - // Canonicalization on both sides must collapse this difference. - // Explicit escape construction — relying on source-literal `é` is - // unreliable because editors/formatters may NFC-normalize on save. - const nfcName = 'qu\u00e9' // 'qué' — single codepoint U+00E9 - const nfdName = 'qu\u0065\u0301' // 'qué' — e + combining acute + it('treats NFC and NFD spellings as distinct byte-faithful targets (renames, not a no-op)', async () => { + // externalPath is stored byte-faithful (no NFC), so an NFC-stored entry + // renamed to the NFD spelling of the same display name is a REAL rename to + // a byte-distinct target, not a short-circuited no-op. Explicit escape + // construction — relying on source-literal `é` is unreliable because + // editors/formatters may NFC-normalize on save. + const nfcName = 'qu\u00e9' // 'qué' — single codepoint U+00E9 (NFC) + const nfdName = 'qu\u0065\u0301' // 'qué' — e + combining acute (NFD) expect(nfcName).not.toBe(nfdName) // byte-distinct strings expect(nfcName.normalize('NFC')).toBe(nfdName.normalize('NFC')) const filePath = path.join(tmp, `${nfcName}.txt`) await writeFile(filePath, 'x') const entry = await ensureExternal(deps, { externalPath: filePath as FilePath }) + if (entry.origin !== 'external') throw new Error('expected external entry') + expect(entry.externalPath).toBe(filePath) // byte-faithful NFC, no fold // Spy on the file module's `move` wrapper, not `node:fs/promises.rename`: // the latter is a Node native ESM namespace member and Vitest cannot @@ -144,15 +145,18 @@ describe('internal/entry/rename', () => { const fsModule = await import('@main/utils/file') const moveSpy = vi.spyOn(fsModule, 'move') - // Re-rename to the NFD form — same logical name, different codepoints. + // Rename to the NFD spelling — same logical name, byte-distinct target. const result = await rename(deps, entry.id, nfdName) - expect(moveSpy).not.toHaveBeenCalled() + // The rename is NOT short-circuited: `move` runs and the DB row takes the + // byte-faithful NFD path (no NFC fold). + expect(moveSpy).toHaveBeenCalled() expect(result.id).toBe(entry.id) if (result.origin !== 'external' || entry.origin !== 'external') { throw new Error('expected external entries') } - expect(result.externalPath).toBe(entry.externalPath) // still NFC-canonical + expect(result.name).toBe(nfdName) + expect(result.externalPath).toBe(path.join(tmp, `${nfdName}.txt`)) // byte-faithful NFD }) it('allows a case-only rename when the existing file at target is the same inode', async () => { diff --git a/src/main/services/file/internal/entry/create.ts b/src/main/services/file/internal/entry/create.ts index 16fc80e05db..e020fb7c646 100644 --- a/src/main/services/file/internal/entry/create.ts +++ b/src/main/services/file/internal/entry/create.ts @@ -16,12 +16,12 @@ import { application } from '@application' import { loggerService } from '@logger' import { atomicWriteFile, copy as fsCopy, download, remove as fsRemove, stat as fsStat } from '@main/utils/file' import type { FileEntry } from '@shared/data/types/file' -import type { FilePath } from '@shared/types/file' +import { type FilePath, FilePathSchema } from '@shared/types/file' +import { canonicalizeFilePath } from '@shared/utils/file' import mime from 'mime' import { v7 as uuidv7 } from 'uuid' import type { CreateInternalEntryParams, EnsureExternalEntryParams } from '../../FileManager' -import { canonicalizeExternalPath } from '../../utils/pathResolver' import type { FileManagerDeps } from '../deps' const logger = loggerService.withContext('internal/entry/create') @@ -139,7 +139,7 @@ export async function createInternal(deps: FileManagerDeps, params: CreateIntern const source = normaliseSource(params) const id = uuidv7() const filename = `${id}${source.ext ? `.${source.ext}` : ''}` - const physical = application.getPath('feature.files.data', filename) as FilePath + const physical = FilePathSchema.parse(application.getPath('feature.files.data', filename)) await source.writeTo(physical) let stats try { @@ -165,74 +165,92 @@ export async function createInternal(deps: FileManagerDeps, params: CreateIntern /** * Ensure an entry exists for a user-provided absolute path. Pure upsert keyed - * by canonicalized externalPath. Path existence is verified via `fs.stat` - * before insert; ENOENT propagates. + * by the canonical form of `params.externalPath`: `FilePathSchema` at the IPC + * boundary validates shape only, so this function canonicalizes the input via + * `canonicalizeFilePath` and derives every downstream value (lookup, dedup, + * name/ext projection, persisted `externalPath`) from that `CanonicalFilePath`. + * + * External entries may be created **dangling** — an external reference to a + * path that is not currently on disk is a first-class state (see + * `DanglingState`), so presence is probed best-effort, never required: + * `fs.stat` succeeding records `'present'`; ENOENT / ENOTDIR ("the file is + * simply not there") records `'missing'` and still inserts a dangling entry; + * any other errno (EACCES, EIO, …) is a genuine FS fault and rethrows. The + * `fs.realpath` case-collision probe needs the file on disk, so it runs only + * when the file is present. */ export async function ensureExternal(deps: FileManagerDeps, params: EnsureExternalEntryParams): Promise { - const canonical = canonicalizeExternalPath(params.externalPath) + const canonical = canonicalizeFilePath(params.externalPath) const existing = deps.fileEntryService.findByExternalPath(canonical) if (existing) return existing - // Every downstream derivation must consume the canonical path, not the - // raw `params.externalPath`. On macOS APFS the raw input can arrive in - // NFD form while `canonical` is NFC; deriving `name` / `ext` from raw - // would persist NFD-encoded values alongside an NFC `externalPath`, so - // a later strict-equality check like `path.basename(canonical) === entry.name` - // would silently diverge. Same risk for trailing-separator / `..` - // noise in the raw input. - // `canonical` is `CanonicalExternalPath`; the schema-side S5 refine now - // makes the BO's `externalPath` `FilePath & CanonicalExternalPath`, but - // here we only hold the factory-side `CanonicalExternalPath`. The cast - // to `FilePath` is the sanctioned service-boundary upcast — the - // canonicalize pipeline already enforces the absolute-shape gate that - // `FilePath` represents at the type level. - await fsStat(canonical as unknown as FilePath) - // Case-insensitive peer lookup is index-backed via the - // `fe_external_path_lower_unique_idx` functional UNIQUE on `lower(externalPath)`. - // The same index hard-rejects an INSERT that would collide with an existing - // peer's lowercased form, so we MUST resolve the collision at the - // application layer before attempting the INSERT — otherwise a legitimate - // distinct-file reference on a case-sensitive filesystem (Linux ext4 / - // case-sensitive APFS volume) would surface as an opaque SQLITE_CONSTRAINT. - // - // Disambiguation strategy: `fs.realpath`. On case-insensitive filesystems - // (macOS APFS default, Windows NTFS default) the FS itself folds case, - // so `realpath('/foo/A.txt')` and `realpath('/foo/a.txt')` return the same - // on-disk canonical string → same logical file, reuse the existing peer. - // On case-sensitive filesystems the two paths resolve to distinct strings - // (or one ENOENTs) → genuine distinct files, throw with peer info so the - // caller can decide (rename / surface to user). This is the `fs.realpath` - // upgrade pre-announced in `canonicalizeExternalPath`'s JSDoc. - // - // SELECT failure (transient DB lock, connection drop) propagates; the - // subsequent INSERT would fail at the same boundary with a more - // diagnosable stack, so wrapping in try/catch here only hides the real - // error one stack frame earlier. - const peers = deps.fileEntryService.findCaseInsensitivePeers(canonical) - if (peers.length > 0) { - const reusable = await resolveCaseCollisionPeer(canonical as FilePath, peers) - if (reusable) { - logger.info('ensureExternal: reusing case-collision peer (fs.realpath confirmed same FS entry)', { - newPath: canonical, - peerId: reusable.id, - peerPath: (reusable as { externalPath: string }).externalPath - }) - return reusable + // Presence probe (best-effort, not a precondition). ENOENT / ENOTDIR mean + // the file is simply absent → create the entry dangling. Any other errno + // (EACCES, EIO, …) is a genuine FS fault, NOT a "dangling" situation, so we + // preserve the old fail-loud behavior and rethrow. Discriminate on the raw + // `.code`, mirroring `danglingCache.ts` `defaultStatProbe` / `pathStatus.ts`. + let present: boolean + try { + await fsStat(canonical) + present = true + } catch (err) { + const code = (err as NodeJS.ErrnoException).code + if (code !== 'ENOENT' && code !== 'ENOTDIR') throw err + present = false + } + // Case-collision resolution requires the file on disk: `fs.realpath` cannot + // probe an absent path, so we gate the whole peer block on `present`. When + // the new path is missing we skip it — a missing path that case-collides + // with an existing entry is still rejected by the DB `lower(externalPath)` + // UNIQUE index at INSERT (SQLITE_CONSTRAINT), which is the correct outcome; + // we simply cannot produce the friendlier pre-INSERT error without the file. + if (present) { + // Case-insensitive peer lookup is index-backed via the + // `fe_external_path_lower_unique_idx` functional UNIQUE on `lower(externalPath)`. + // The same index hard-rejects an INSERT that would collide with an existing + // peer's lowercased form, so we MUST resolve the collision at the + // application layer before attempting the INSERT — otherwise a legitimate + // distinct-file reference on a case-sensitive filesystem (Linux ext4 / + // case-sensitive APFS volume) would surface as an opaque SQLITE_CONSTRAINT. + // + // Disambiguation strategy: `fs.realpath`. On case-insensitive filesystems + // (macOS APFS default, Windows NTFS default) the FS itself folds case, + // so `realpath('/foo/A.txt')` and `realpath('/foo/a.txt')` return the same + // on-disk canonical string → same logical file, reuse the existing peer. + // On case-sensitive filesystems the two paths resolve to distinct strings + // (or one ENOENTs) → genuine distinct files, throw with peer info so the + // caller can decide (rename / surface to user). + // + // SELECT failure (transient DB lock, connection drop) propagates; the + // subsequent INSERT would fail at the same boundary with a more + // diagnosable stack, so wrapping in try/catch here only hides the real + // error one stack frame earlier. + const peers = deps.fileEntryService.findCaseInsensitivePeers(canonical) + if (peers.length > 0) { + const reusable = await resolveCaseCollisionPeer(canonical, peers) + if (reusable) { + logger.info('ensureExternal: reusing case-collision peer (fs.realpath confirmed same FS entry)', { + newPath: canonical, + peerId: reusable.id, + peerPath: (reusable as { externalPath: string }).externalPath + }) + return reusable + } + // No peer is the same FS entity. On a case-sensitive filesystem these + // are legitimately distinct files, but the DB unique constraint forbids + // the insert. Throw with full peer detail so the caller can act + // (rename one of the colliding paths, or surface the conflict to the + // user). This is a deliberate departure from the previous "warn-only" + // contract — the application-layer hard guarantee on lowered-path + // uniqueness is what option (c) brings. + throw new Error( + `ensureExternal: case-collision with existing entries — fs.realpath confirms different FS entities. ` + + `New: ${canonical}; conflicting peers: ${peers + .map((p) => `${p.id}=${(p as { externalPath: string }).externalPath}`) + .join(', ')}` + ) } - // No peer is the same FS entity. On a case-sensitive filesystem these - // are legitimately distinct files, but the DB unique constraint forbids - // the insert. Throw with full peer detail so the caller can act - // (rename one of the colliding paths, or surface the conflict to the - // user). This is a deliberate departure from the previous "warn-only" - // contract — the application-layer hard guarantee on lowered-path - // uniqueness is what option (c) brings. - throw new Error( - `ensureExternal: case-collision with existing entries — fs.realpath confirms different FS entities. ` + - `New: ${canonical}; conflicting peers: ${peers - .map((p) => `${p.id}=${(p as { externalPath: string }).externalPath}`) - .join(', ')}` - ) } - // `name` and `ext` are pure projections of `canonical` — derived here, + // `name` and `ext` are pure projections of `externalPath` — derived here, // not accepted from callers. Doc-stated invariant: "external `name` is a // pure projection of `externalPath`" (file-manager-architecture §1.5 + // architecture §3.3) is now enforced by the IPC type lacking a `name` @@ -247,11 +265,11 @@ export async function ensureExternal(deps: FileManagerDeps, params: EnsureExtern externalPath: canonical }) // Reverse-index hook: subsequent watcher / opportunistic ops events for - // `canonical` should reach this entry id. The fs.stat above succeeded — - // record a fresh 'present' observation so any imminent UI query short- - // circuits the cold-stat path. - deps.danglingCache.addEntry(inserted.id, canonical as FilePath) - deps.danglingCache.onFsEvent(canonical as FilePath, 'present', 'ops') + // `canonical` should reach this entry id. Record the probed presence as a + // fresh 'present' / 'missing' observation so any imminent UI query + // short-circuits the cold-stat path (a dangling create seeds 'missing'). + deps.danglingCache.addEntry(inserted.id, canonical) + deps.danglingCache.onFsEvent(canonical, present ? 'present' : 'missing', 'ops') return inserted } diff --git a/src/main/services/file/internal/entry/rename.ts b/src/main/services/file/internal/entry/rename.ts index 9b2edf636bf..d4082dd4547 100644 --- a/src/main/services/file/internal/entry/rename.ts +++ b/src/main/services/file/internal/entry/rename.ts @@ -14,9 +14,8 @@ import { loggerService } from '@logger' import { exists, isSameFile, move as fsMove } from '@main/utils/file' import type { FileEntry, FileEntryId } from '@shared/data/types/file' import { SafeNameSchema } from '@shared/data/types/file' -import type { FilePath } from '@shared/types/file' +import { canonicalizeFilePath } from '@shared/utils/file' -import { canonicalizeExternalPath } from '../../utils/pathResolver' import type { FileManagerDeps } from '../deps' const logger = loggerService.withContext('internal/entry/rename') @@ -34,17 +33,18 @@ export async function rename(deps: FileManagerDeps, id: FileEntryId, newName: st return deps.fileEntryService.update(id, { name: newName }) } // entry.origin === 'external' from here on; the schema discriminator - // guarantees externalPath is present as `AbsolutePathSchema` (no `null` + // guarantees externalPath is present as `FilePathSchema` (no `null` // branch exists on ExternalEntrySchema), so the prior defensive // `if (!entry.externalPath)` throw was unreachable and has been removed. const dir = path.dirname(entry.externalPath) const ext = entry.ext ? `.${entry.ext}` : '' - // Canonicalize the target so the no-op check below tolerates NFC/NFD, - // trailing-separator, and `.`/`..` noise — `entry.externalPath` is already - // canonical (written through `ensureExternalEntry`), so string equality - // here is a reliable "same logical path" test. + // Canonicalize the target so the no-op check below tolerates trailing- + // separator and `.`/`..` noise — `entry.externalPath` is already canonical + // (byte-faithful, written through `ensureExternalEntry`), so string equality + // here is a reliable "same byte path" test. Canonicalization is byte-faithful + // (no NFC), so NFC and NFD spellings of a name are distinct targets. const oldPath = entry.externalPath - const target = canonicalizeExternalPath(path.join(dir, `${newName}${ext}`)) + const target = canonicalizeFilePath(path.join(dir, `${newName}${ext}`)) // Defense in depth: `SafeNameSchema.parse(newName)` above already rejects // path separators and `..`, but a future regression in the schema (or any // canonicalization behaviour change) must not be able to relocate the @@ -55,17 +55,17 @@ export async function rename(deps: FileManagerDeps, id: FileEntryId, newName: st if (target === entry.externalPath) { return entry } - if (await exists(target as FilePath)) { + if (await exists(target)) { // On case-insensitive filesystems (macOS APFS / Windows NTFS) a // `Foo.pdf → foo.pdf` rename hits this branch because `exists` reports // the file under its on-disk case. If both paths resolve to the same // inode it's a legitimate case-only rename — fall through to `fsMove`, // which the OS treats as an in-place case fix. - if (!(await isSameFile(target as FilePath, oldPath))) { + if (!(await isSameFile(target, oldPath))) { throw new Error(`rename: target path already exists: ${target}`) } } - await fsMove(oldPath, target as FilePath) + await fsMove(oldPath, target) const canonical = target // Single atomic DB write — `setExternalPathAndName` is the only sanctioned // mutation site for `externalPath`. Doing both column changes in one @@ -86,7 +86,7 @@ export async function rename(deps: FileManagerDeps, id: FileEntryId, newName: st renamed = deps.fileEntryService.setExternalPathAndName(id, canonical, newName) } catch (dbErr) { try { - await fsMove(canonical as FilePath, oldPath) + await fsMove(canonical, oldPath) } catch (rollbackErr) { logger.warn('rename: FS-DB skew — file moved to target but DB update failed, and rollback failed', { id, @@ -107,7 +107,7 @@ export async function rename(deps: FileManagerDeps, id: FileEntryId, newName: st // Reverse-index swap. The old path is fully invalidated; the new path // takes over with a fresh 'present' observation since fsMove just succeeded. deps.danglingCache.removeEntry(id, oldPath) - deps.danglingCache.addEntry(id, canonical as FilePath) - deps.danglingCache.onFsEvent(canonical as FilePath, 'present', 'ops') + deps.danglingCache.addEntry(id, canonical) + deps.danglingCache.onFsEvent(canonical, 'present', 'ops') return renamed } diff --git a/src/main/services/file/internal/system/tempCopy.ts b/src/main/services/file/internal/system/tempCopy.ts index 356f258ecd7..27bf6560a53 100644 --- a/src/main/services/file/internal/system/tempCopy.ts +++ b/src/main/services/file/internal/system/tempCopy.ts @@ -15,7 +15,7 @@ import { application } from '@application' import { loggerService } from '@logger' import { copy as fsCopy, removeDir as fsRemoveDir } from '@main/utils/file' import type { FileEntryId } from '@shared/data/types/file' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { resolvePhysicalPath } from '../../utils/pathResolver' import type { FileManagerDeps } from '../deps' @@ -35,7 +35,7 @@ export async function withTempCopy( const parent = application.getPath('feature.files.tempcopy.temp') const dir = await mkdtemp(path.join(parent, 'tc-')) const filename = `${entry.name}${entry.ext ? `.${entry.ext}` : ''}` || 'file' - const target = path.join(dir, filename) as FilePath + const target = FilePathSchema.parse(path.join(dir, filename)) try { await fsCopy(physical, target) return await fn(target) @@ -50,7 +50,7 @@ export async function withTempCopy( // on the next OS-level temp cleanup. No application-side sweeper is // planned. try { - await fsRemoveDir(dir as FilePath) + await fsRemoveDir(FilePathSchema.parse(dir)) } catch (cleanupErr) { logger.warn('withTempCopy: temp dir cleanup failed; directory will leak until OS temp reap', { dir, diff --git a/src/main/services/file/toFileInfo.ts b/src/main/services/file/toFileInfo.ts index fc95bb2d09e..1d088b05187 100644 --- a/src/main/services/file/toFileInfo.ts +++ b/src/main/services/file/toFileInfo.ts @@ -37,10 +37,9 @@ export async function toFileInfo(entry: FileEntry): Promise { const s = await fsStat(physicalPath) const ext = entry.ext const inferredMime = ext ? (mime.getType(ext) ?? 'application/octet-stream') : 'application/octet-stream' - // `FileInfoSchema.parse` rehydrates the `FileInfo` brand. Casting back to - // `FileInfo` lets the `path` field carry the `FilePath` template-literal - // type at the API surface (Zod can't express template literals); the - // runtime shape check is otherwise identical. + // `FileInfoSchema.parse` validates the shape and returns the exact `FileInfo` + // type (including the branded `FilePath` on `path`) — no `as FileInfo` cast + // needed. return FileInfoSchema.parse({ path: physicalPath, name: entry.name, @@ -50,5 +49,5 @@ export async function toFileInfo(entry: FileEntry): Promise { type: getFileTypeByExt(ext ?? ''), createdAt: s.createdAt || s.modifiedAt, modifiedAt: s.modifiedAt - }) as FileInfo + }) } diff --git a/src/main/services/file/tree/DirectoryTreeManager.ts b/src/main/services/file/tree/DirectoryTreeManager.ts index b4f61f11ccc..125ddb6c24a 100644 --- a/src/main/services/file/tree/DirectoryTreeManager.ts +++ b/src/main/services/file/tree/DirectoryTreeManager.ts @@ -27,8 +27,8 @@ import { randomUUID } from 'node:crypto' import { loggerService } from '@logger' import { BaseService, type Disposable, Injectable, Phase, ServicePhase } from '@main/core/lifecycle' -import { AbsolutePathSchema } from '@shared/data/types/file' import { IpcChannel } from '@shared/IpcChannel' +import { FilePathSchema } from '@shared/types/file' import { type CreateTreeIpcResult, type DirectoryTreeOptions, @@ -45,7 +45,7 @@ import { createDirectoryTree, type DirectoryTreeBuilder } from './builder' // next to the handlers, matching the FileManager / DataApi convention where // leaf schemas live in shared and per-channel param schemas live in main. const TreeCreateParamsSchema = z.strictObject({ - rootPath: AbsolutePathSchema, + rootPath: FilePathSchema, options: DirectoryTreeOptionsSchema.optional() }) @@ -53,8 +53,8 @@ const TreeDisposeParamsSchema = z.strictObject({ treeId: z.string().min(1) }) const TreeRenameParamsSchema = z.strictObject({ treeId: z.string().min(1), - oldPath: AbsolutePathSchema, - newPath: AbsolutePathSchema + oldPath: FilePathSchema, + newPath: FilePathSchema }) /** diff --git a/src/main/services/file/utils/__tests__/pathResolver.test.ts b/src/main/services/file/utils/__tests__/pathResolver.test.ts index 7aff595ee25..0a6493e1a2a 100644 --- a/src/main/services/file/utils/__tests__/pathResolver.test.ts +++ b/src/main/services/file/utils/__tests__/pathResolver.test.ts @@ -8,12 +8,10 @@ vi.mock('@application', async () => { return mockApplicationFactory() }) -import path from 'node:path' - -import type { CanonicalExternalPath } from '@shared/data/types/file' +import type { FilePath } from '@shared/types/file' import type { PathResolvableEntry } from '../pathResolver' -import { canonicalizeExternalPath, getExtSuffix, resolvePhysicalPath } from '../pathResolver' +import { getExtSuffix, resolvePhysicalPath } from '../pathResolver' describe('getExtSuffix', () => { it('returns dot-prefixed extension for non-null ext', () => { @@ -53,20 +51,10 @@ describe('resolvePhysicalPath', () => { id: '019606a0-0000-7000-8000-000000000001', origin: 'external', ext: 'md', - externalPath: '/Users/me/notes/readme.md' + externalPath: '/Users/me/notes/readme.md' as FilePath } expect(resolvePhysicalPath(entry)).toBe('/Users/me/notes/readme.md') }) - - it('resolves path (normalizes relative segments)', () => { - const entry: PathResolvableEntry = { - id: '019606a0-0000-7000-8000-000000000002', - origin: 'external', - ext: 'pdf', - externalPath: '/Users/me/./docs/../docs/report.pdf' - } - expect(resolvePhysicalPath(entry)).toBe('/Users/me/docs/report.pdf') - }) }) describe('security', () => { @@ -87,46 +75,5 @@ describe('resolvePhysicalPath', () => { } expect(() => resolvePhysicalPath(entry)).toThrow('null bytes') }) - - it('rejects null bytes in externalPath', () => { - const entry: PathResolvableEntry = { - id: '019606a0-0000-7000-8000-000000000001', - origin: 'external', - ext: 'md', - externalPath: '/Users/me/evil\0path.md' - } - expect(() => resolvePhysicalPath(entry)).toThrow('null bytes') - }) - }) -}) - -describe('canonicalizeExternalPath', () => { - it('resolves "." and ".." segments', () => { - const input = path.resolve('/foo/./bar/../baz') - expect(canonicalizeExternalPath(input) as string).toBe(path.resolve('/foo/baz')) - }) - - it('NFC-normalizes Unicode (NFD → NFC)', () => { - const nfd = '/users/Müller' - const nfc = '/users/Müller' - expect(canonicalizeExternalPath(nfd) as string).toBe(canonicalizeExternalPath(nfc) as string) - }) - - it('strips trailing path separator', () => { - expect(canonicalizeExternalPath('/foo/bar/') as string).toBe(canonicalizeExternalPath('/foo/bar') as string) - }) - - it('preserves a path that is already canonical', () => { - const canonical = path.resolve('/foo/bar/baz.txt') - expect(canonicalizeExternalPath(canonical) as string).toBe(canonical) - }) - - it('rejects null bytes', () => { - expect(() => canonicalizeExternalPath('/foo/bar\0/baz')).toThrow(/null byte/i) - }) - - it('returns a CanonicalExternalPath brand (compile-time check)', () => { - const canonical: CanonicalExternalPath = canonicalizeExternalPath('/foo') - expect(typeof canonical).toBe('string') }) }) diff --git a/src/main/services/file/utils/pathResolver.ts b/src/main/services/file/utils/pathResolver.ts index 274c1bc42ec..acd044707de 100644 --- a/src/main/services/file/utils/pathResolver.ts +++ b/src/main/services/file/utils/pathResolver.ts @@ -1,10 +1,6 @@ -import path from 'node:path' - import { application } from '@application' import { loggerService } from '@logger' -import type { CanonicalExternalPath } from '@shared/data/types/file' -import type { FilePath } from '@shared/types/file' -import { canonicalizeAbsolutePath } from '@shared/utils/file' +import { type FilePath, FilePathSchema } from '@shared/types/file' const logger = loggerService.withContext('pathResolver') @@ -16,7 +12,7 @@ const logger = loggerService.withContext('pathResolver') */ export type PathResolvableEntry = | { id: string; origin: 'internal'; ext: string | null } - | { id: string; origin: 'external'; ext: string | null; externalPath: string } + | { id: string; origin: 'external'; ext: string | null; externalPath: FilePath } /** * Get the file extension suffix (with dot) or empty string if null. @@ -32,15 +28,15 @@ export function getExtSuffix(ext: string | null): string { * - `origin='external'` → `externalPath` directly (user-provided absolute path) * * Returns a branded `FilePath` so callers can pass the result straight to - * `@main/utils/file/fs` without a manual `as FilePath` cast — the brand - * is sanctioned here because the function ran the null-byte + absolute-path - * guards that make a string safe to treat as a `FilePath`. + * `@main/utils/file/fs` without a manual `as FilePath` cast. * * @throws If null bytes are detected (potential path-truncation attack) in - * entry id / ext / externalPath. Security-sensitive rejections are logged - * at `error` level — these inputs should never reach the resolver if - * upstream Zod validation runs; arriving here indicates either a - * parse-bypass or a data integrity problem worth investigating. + * entry id / ext. Security-sensitive rejections are logged at `error` + * level — these inputs should never reach the resolver if upstream Zod + * validation runs; arriving here indicates either a parse-bypass or a data + * integrity problem worth investigating. The external branch cannot throw + * this way: `entry.externalPath` is already a `FilePath`, proven canonical + * (including null-byte rejection) by `FilePathSchema` at parse time. */ export function resolvePhysicalPath(entry: PathResolvableEntry): FilePath { // Reject null bytes in any user-controlled path segments (path-truncation guard). @@ -50,108 +46,11 @@ export function resolvePhysicalPath(entry: PathResolvableEntry): FilePath { } if (entry.origin === 'internal') { - return application.getPath('feature.files.data', `${entry.id}${getExtSuffix(entry.ext)}`) as FilePath + return FilePathSchema.parse(application.getPath('feature.files.data', `${entry.id}${getExtSuffix(entry.ext)}`)) } - // entry.origin === 'external' — schema discriminator guarantees externalPath - // is present (no null branch on ExternalEntrySchema), so the prior - // defensive null-check is unreachable and has been removed. - if (entry.externalPath.includes('\0')) { - logger.error('Null byte detected in externalPath', { entryId: entry.id }) - throw new Error(`external entry ${entry.id} externalPath contains null bytes`) - } - return path.resolve(entry.externalPath) as FilePath -} - -/** - * Canonicalize a user-provided external path into the form stored in - * `file_entry.externalPath`. This result is the **sole** key used for upsert - * and lookup of external entries — `ensureExternalEntry` MUST call this - * before any DB write or query, and `fileEntryService.findByExternalPath` - * does the same at read boundaries. - * - * The return type is branded as `CanonicalExternalPath` so that downstream - * surfaces filtering by `externalPath` (today: `findByExternalPath`; in future: - * any new DataApi / service query on this column) cannot accept a raw user - * path by mistake. New call sites gain compile-time guarding for free. - * - * ───────────────────────────────────────────────────────────────────────── - * ⚠️ RULE-EVOLUTION DISCIPLINE - * ───────────────────────────────────────────────────────────────────────── - * Modifying the normalization behavior of this function (adding / removing / - * altering any step below) REQUIRES a paired Drizzle migration that - * re-canonicalizes every existing `file_entry` row where `origin='external'`. - * - * Rationale: the canonical form is application-layer logic, not DB schema. - * Existing rows were written under the **old** rule; new queries run under - * the **new** rule. Without a re-canonicalization migration, historical rows - * remain under the old rule and silently stop matching `findByExternalPath` - * lookups — users experience "my file is in the library but the system says - * it isn't". - * - * When a new rule also collapses previously-distinct strings to the same - * canonical form (e.g. `fs.realpath` merging case-insensitive duplicates), - * the migration additionally MUST merge the colliding rows. The winner - * selection and association-row re-pointing rules are defined in - * `docs/references/file/file-manager-architecture.md §1.2 Rule evolution - * discipline` — follow them exactly; do not improvise. - * ───────────────────────────────────────────────────────────────────────── - * - * ## Scope (this function's contract) - * - * Cheap, synchronous normalization — no FS IO, cross-platform uniform: - * - * 0. Reject null bytes (`\0`) — a null byte in the raw path is never a - * legal filesystem path; rejecting at canonicalization keeps the rest - * of the pipeline (ensureExternalEntry → findByExternalPath → - * resolvePhysicalPath) free to treat canonical paths as null-byte-free. - * Without this step, a malformed path could be persisted into - * `file_entry.externalPath` and only blow up later at use-time inside - * `resolvePhysicalPath`, leaving a poisoned row in the DB. - * 1. `path.resolve(raw)` — make absolute, resolve `./` and `../` - * 2. Unicode NFC normalization — defends against NFC-vs-NFD mismatch - * (macOS filesystems can surface either form depending on API; this - * is the most common duplicate-entry trigger for CJK-filename users) - * 3. Strip trailing separator — `/foo/bar/` → `/foo/bar` - * - * This subset covers the dominant sources of `externalPath` inputs in Cherry - * (Electron `showOpenDialog` and drag-drop, both of which return OS-canonical - * paths with correct case already). - * - * ## Deliberately NOT handled here - * - * The following are handled elsewhere, NOT by this function — `canonicalizeExternalPath` - * stays sync / FS-IO-free so it can run from Zod refines and read paths without - * paying `fs.*` cost on every call. - * - * - **Case-insensitive FS de-duplication** (macOS APFS / Windows NTFS): - * `/Users/me/FILE.pdf` vs `/Users/me/file.pdf` are byte-distinct after - * canonicalize and would still produce two rows under this function alone. - * Enforced at two layers downstream: (1) DB functional unique index - * `UNIQUE(lower(externalPath))` rejects the second row at INSERT, and - * (2) `ensureExternalEntry` runs `fs.realpath` on the colliding pair - * before INSERT and either reuses the existing row (same on-disk entity, - * case-insensitive FS) or throws `case-collision` (distinct files, - * case-sensitive FS). See `file-manager-architecture.md §1.2 Duplicate-entry - * detection on insert`. - * - **Symlink resolution** (`realpath` target collapse): two symlinks to the - * same file remain distinct entries. Not enforced anywhere — symlink - * collapse would require unconditional `fs.realpath` on every canonicalize, - * trading the sync/cheap contract this function commits to. - * - **Windows short-name (8.3) resolution**: `LONGNA~1` vs `longname` — - * requires WinAPI; low-priority edge case. - * - **SMB / NFS mounts with FS-level case-sensitivity diverging from host**: - * out of scope; document as known limitation. - * - * - * @param raw user-provided absolute (or resolvable) path - * @returns canonical form stored in `file_entry.externalPath` - * @throws if `raw` contains null bytes - */ -export function canonicalizeExternalPath(raw: string): CanonicalExternalPath { - // Delegate to the shared pure-JS implementation so the FileEntry schema - // can `refine` against the exact same rule on parse (S5). The brand cast - // here is the sanctioned production-side factory site documented in the - // `CanonicalExternalPath` JSDoc. - return canonicalizeAbsolutePath(raw) as CanonicalExternalPath + // entry.origin === 'external' — externalPath is already a canonical + // FilePath (branded at parse time by FilePathSchema), so no further + // normalization or null-byte check is needed here. + return entry.externalPath } diff --git a/src/main/services/file/watcher.ts b/src/main/services/file/watcher.ts index ee4f06a25b6..288060d48e6 100644 --- a/src/main/services/file/watcher.ts +++ b/src/main/services/file/watcher.ts @@ -37,7 +37,7 @@ import path from 'node:path' import { loggerService } from '@logger' import { Emitter } from '@main/core/lifecycle' -import type { FilePath } from '@shared/types/file' +import { type FilePath, FilePathSchema } from '@shared/types/file' import { type FSWatcher, watch as chokidarWatch } from 'chokidar' import { danglingCache } from './danglingCache' @@ -66,6 +66,12 @@ export type WatcherEvent = export type WatcherListener = (event: WatcherEvent) => void +/** Internal shape before FilePath validation. */ +type RawWatcherPathEvent = { + readonly kind: 'add' | 'addDir' | 'unlink' | 'unlinkDir' | 'change' + readonly path: string +} + export interface DirectoryWatcher { /** * Subscribe to normalized FS events. Returns an unsubscribe function. @@ -116,18 +122,26 @@ class DirectoryWatcherImpl implements DirectoryWatcher { const depth = recursive ? this.opts.maxDepth : 0 const fsw = chokidarWatch(this.root, { - ignored: userIgnore ? [builtinIgnore, (p) => userIgnore(p as FilePath)] : [builtinIgnore], + ignored: userIgnore + ? [ + builtinIgnore, + (p) => { + const parsed = this.parseChokidarPath(p) + return !parsed || userIgnore(parsed) + } + ] + : [builtinIgnore], ignoreInitial: true, depth, awaitWriteFinish: stability > 0 ? { stabilityThreshold: stability, pollInterval: 100 } : false, usePolling }) - fsw.on('add', (p) => this.handle({ kind: 'add', path: p as FilePath })) - fsw.on('addDir', (p) => this.handle({ kind: 'addDir', path: p as FilePath })) - fsw.on('change', (p) => this.handle({ kind: 'change', path: p as FilePath })) - fsw.on('unlink', (p) => this.handle({ kind: 'unlink', path: p as FilePath })) - fsw.on('unlinkDir', (p) => this.handle({ kind: 'unlinkDir', path: p as FilePath })) + fsw.on('add', (p) => this.handle({ kind: 'add', path: p })) + fsw.on('addDir', (p) => this.handle({ kind: 'addDir', path: p })) + fsw.on('change', (p) => this.handle({ kind: 'change', path: p })) + fsw.on('unlink', (p) => this.handle({ kind: 'unlink', path: p })) + fsw.on('unlinkDir', (p) => this.handle({ kind: 'unlinkDir', path: p })) fsw.on('ready', () => this.emitter.fire({ kind: 'ready' })) fsw.on('error', (err) => this.handleError(err as Error)) @@ -155,31 +169,56 @@ class DirectoryWatcherImpl implements DirectoryWatcher { } } + /** + * Validate a path emitted by chokidar. chokidar's public types only promise + * `string`, but in our configuration (absolute root, no `cwd`) the emitted + * paths should always satisfy `FilePathSchema`. This helper turns that + * assumption into a checked invariant and returns `null` for any unexpected + * value so callers can drop or ignore it safely. + */ + private parseChokidarPath(raw: string): FilePath | null { + const result = FilePathSchema.safeParse(raw) + return result.success ? result.data : null + } + /** * Forward chokidar's `add` / `unlink` / `change` events to subscribers AND * mirror presence transitions into DanglingCache. `change` is intentionally * not mirrored — the file is still present; only mtime drift, which the * cache doesn't track. * - * The cache feed is keyed by canonical (NFC) path because `DanglingCache`'s - * reverse index is populated by `ensureExternalEntry` → `canonicalizeExternalPath` - * (NFC). chokidar emits whatever the OS hands it; on macOS APFS that is NFD - * for CJK / accented filenames migrated from HFS+ (or written by tools like - * `rsync -E` that preserve the source encoding). Without normalizing here - * the `path → entryIds` lookup misses and the cache stays stale. The - * outbound `emitter.fire(ev)` keeps the raw OS path so external subscribers - * (e.g. opening the file with the same string chokidar saw) stay coherent - * with what the FS actually has — only the DanglingCache leg gets the NFC - * form, since it is the only leg that compares against canonical keys. + * FilePath validation happens here, at the boundary between chokidar's + * `string` events and the FilePath-typed consumers (`DanglingCache`, + * `WatcherEvent` subscribers). Invalid paths are logged and dropped. + * + * DanglingCache's reverse index is keyed **byte-faithful**: it is populated + * by `ensureExternalEntry`, whose `externalPath` is stored exactly as the OS + * handed it (byte-faithful lexical form, no NFC). So the watcher matches + * chokidar events by **raw byte equality** — no normalization on either leg. + * (The NFC step that used to sit here existed only to bridge to the old + * NFC-canonical keys; it is removed together with them.) On Linux the raw + * event byte-matches the stored key by construction; on macOS/Windows it + * matches when the path source and chokidar agree on Unicode form. + * + * `check()` — which stats the byte-faithful stored path directly — is the + * correctness baseline, so a missed watcher match is never a correctness + * bug: it only delays a badge update until the next `check()`, which is + * benign and self-healing. See + * `docs/references/file/file-manager-architecture.md §11.3 "Watcher + * Auto-Wiring"`. */ - private handle(ev: Extract): void { + private handle(ev: RawWatcherPathEvent): void { if (this.closed) return + const path = this.parseChokidarPath(ev.path) + if (!path) { + logger.warn('chokidar emitted a path that does not satisfy FilePathSchema', { path: ev.path }) + return + } if (ev.kind === 'add' || ev.kind === 'unlink') { - const canonical = ev.path.normalize('NFC') as FilePath const presence = ev.kind === 'add' ? 'present' : 'missing' - danglingCache.onFsEvent(canonical, presence, 'watcher') + danglingCache.onFsEvent(path, presence, 'watcher') } - this.emitter.fire(ev) + this.emitter.fire({ kind: ev.kind, path }) } onEvent(listener: WatcherListener): () => void { diff --git a/src/main/utils/file/__tests__/pathStatus.test.ts b/src/main/utils/file/__tests__/pathStatus.test.ts index 69082f39dc8..72a766d0671 100644 --- a/src/main/utils/file/__tests__/pathStatus.test.ts +++ b/src/main/utils/file/__tests__/pathStatus.test.ts @@ -38,4 +38,8 @@ describe('getPathStatus', () => { it('short-circuits a blank path to missing without touching the filesystem', async () => { await expect(getPathStatus(' ')).resolves.toEqual({ ok: false, reason: 'missing' }) }) + + it('reports missing for a non-absolute path (fails FilePathSchema, not inaccessible)', async () => { + await expect(getPathStatus('relative/path')).resolves.toEqual({ ok: false, reason: 'missing' }) + }) }) diff --git a/src/main/utils/file/pathStatus.ts b/src/main/utils/file/pathStatus.ts index 77d0d3c05a2..af064f26355 100644 --- a/src/main/utils/file/pathStatus.ts +++ b/src/main/utils/file/pathStatus.ts @@ -1,4 +1,4 @@ -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { stat } from './fs' @@ -26,8 +26,18 @@ export async function getPathStatus(path: string): Promise { return { ok: false, reason: 'missing' } } + // Parse OUTSIDE the try: a non-absolute / relative / `~` / `file://` input + // fails `FilePathSchema` with a ZodError (no `.code`), which would otherwise + // fall into the `catch` below and be misclassified as `inaccessible`. Such + // an input can never resolve to a real path, so it gets the same `missing` + // verdict as an ENOENT. + const parsed = FilePathSchema.safeParse(path) + if (!parsed.success) { + return { ok: false, reason: 'missing' } + } + try { - const stats = await stat(path as FilePath) + const stats = await stat(parsed.data) return { ok: true, kind: stats.isDirectory ? 'directory' : 'file' } } catch (error) { // `ENOENT` (nothing there) and `ENOTDIR` (a path component is a diff --git a/src/renderer/components/chat/messages/hooks/__tests__/useMessageLeafCapabilities.test.tsx b/src/renderer/components/chat/messages/hooks/__tests__/useMessageLeafCapabilities.test.tsx index 7294d81e904..ac0f0adaf81 100644 --- a/src/renderer/components/chat/messages/hooks/__tests__/useMessageLeafCapabilities.test.tsx +++ b/src/renderer/components/chat/messages/hooks/__tests__/useMessageLeafCapabilities.test.tsx @@ -12,10 +12,18 @@ vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) -const { mockUseExternalApps, mockPreview, mockSafeOpen } = vi.hoisted(() => ({ +const { mockUseExternalApps, mockPreview, mockSafeOpen, mockLoggerWarn, mockLoggerDebug } = vi.hoisted(() => ({ mockUseExternalApps: vi.fn(() => ({ data: [] })), mockPreview: vi.fn(), - mockSafeOpen: vi.fn() + mockSafeOpen: vi.fn(), + mockLoggerWarn: vi.fn(), + mockLoggerDebug: vi.fn() +})) + +vi.mock('@logger', () => ({ + loggerService: { + withContext: () => ({ warn: mockLoggerWarn, debug: mockLoggerDebug }) + } })) vi.mock('@renderer/hooks/useAttachment', () => ({ @@ -147,6 +155,10 @@ describe('useMessageLeafCapabilities', () => { await result.current.openFile?.(file) expect(mockSafeOpen).toHaveBeenCalledWith({ kind: 'entry', entryId: '019606a0-0000-7000-8000-000000000001' }) + expect(mockLoggerDebug).toHaveBeenCalledWith( + 'fileMetadataToHandle: falling back to entry id for non-absolute path', + expect.objectContaining({ path: 'relative/legacy.pdf' }) + ) }) it('projects file display data for shared attachment renderers', () => { @@ -180,6 +192,32 @@ describe('useMessageLeafCapabilities', () => { }) }) + it('omits the preview url without throwing when the shared attachment path is not absolute', () => { + const { result } = renderHook(() => useMessageLeafCapabilities({ partsByMessageId: {} })) + + const file: FileMetadata = { + id: 'file-1', + type: FILE_TYPE.DOCUMENT, + ext: '.pdf', + path: 'relative/legacy.pdf', + origin_name: 'file.pdf', + name: 'stored-file.pdf', + size: 100, + created_at: '2026-01-01T00:00:00.000Z', + count: 1 + } + + expect(() => result.current.getFileView?.(file)).not.toThrow() + expect(result.current.getFileView?.(file)).toEqual({ + displayName: 'file.pdf', + previewUrl: undefined + }) + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'getFileView: non-canonical/invalid attachment path', + expect.objectContaining({ path: 'relative/legacy.pdf' }) + ) + }) + it('keeps legacy pasted temp-file display behavior local to message attachments', () => { const { result } = renderHook(() => useMessageLeafCapabilities({ partsByMessageId: {} })) diff --git a/src/renderer/components/chat/messages/hooks/useMessageLeafCapabilities.ts b/src/renderer/components/chat/messages/hooks/useMessageLeafCapabilities.ts index 83b175b9bf0..17bd1d1ed93 100644 --- a/src/renderer/components/chat/messages/hooks/useMessageLeafCapabilities.ts +++ b/src/renderer/components/chat/messages/hooks/useMessageLeafCapabilities.ts @@ -1,4 +1,5 @@ import { useQuery } from '@data/hooks/useDataApi' +import { loggerService } from '@logger' import type { MessageListActions, MessageListState } from '@renderer/components/chat/messages/types' import { useAttachment } from '@renderer/hooks/useAttachment' import { useExternalApps } from '@renderer/hooks/useExternalApps' @@ -11,7 +12,7 @@ import { safeOpen } from '@renderer/utils/file/safeOpen' import type { FileHandle } from '@shared/data/types/file' import type { CherryMessagePart } from '@shared/data/types/message' import { IpcChannel } from '@shared/IpcChannel' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import type { McpProgressEvent } from '@shared/types/mcp' import { createFileEntryHandle, createFilePathHandle, toSafeFileUrl } from '@shared/utils/file' import dayjs from 'dayjs' @@ -21,6 +22,8 @@ import { useTranslation } from 'react-i18next' import { type MessagePlatformActions, useMessagePlatformActions } from './useMessagePlatformActions' +const logger = loggerService.withContext('useMessageLeafCapabilities') + type MessageLeafActions = Pick< MessageListActions, 'previewFile' | 'openFile' | 'subscribeToolProgress' | 'openExternalUrl' | 'openInExternalApp' @@ -55,10 +58,14 @@ function isMcpToolPart(part: CherryMessagePart): boolean { function fileMetadataToHandle(file: FileMetadata): FileHandle { if (file.path) { try { - return createFilePathHandle(file.path as FilePath) + return createFilePathHandle(FilePathSchema.parse(file.path)) } catch { // Fall back to the entry id for legacy FileMetadata whose path is not an // absolute filesystem path. The IPC schema is still the authority. + logger.debug('fileMetadataToHandle: falling back to entry id for non-absolute path', { + fileId: file.id, + path: file.path + }) } } @@ -136,9 +143,13 @@ export function useMessageLeafCapabilities({ const getFileView = useCallback>( (file) => { + const parsedPath = file.path ? FilePathSchema.safeParse(file.path) : undefined + if (parsedPath && !parsedPath.success) { + logger.warn('getFileView: non-canonical/invalid attachment path', { fileId: file.id, path: file.path }) + } return { displayName: formatMessageAttachmentFileName(file, t), - previewUrl: file.path ? toSafeFileUrl(file.path as FilePath, file.ext || null) : undefined + previewUrl: parsedPath?.success ? toSafeFileUrl(parsedPath.data, file.ext || null) : undefined } }, [t] diff --git a/src/renderer/components/chat/panes/ArtifactPane.tsx b/src/renderer/components/chat/panes/ArtifactPane.tsx index 59d389682bc..0ca80b02e3a 100644 --- a/src/renderer/components/chat/panes/ArtifactPane.tsx +++ b/src/renderer/components/chat/panes/ArtifactPane.tsx @@ -19,7 +19,7 @@ import { buildEditorUrl } from '@renderer/utils/editor' import { formatErrorMessageWithPrefix } from '@renderer/utils/error' import { joinPath } from '@renderer/utils/path' import { isMac, isWin } from '@renderer/utils/platform' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { toFileUrl } from '@shared/utils/file' import { AlertCircle, FileText, FolderOpen, RotateCw, Sparkles, X } from 'lucide-react' import { @@ -125,6 +125,10 @@ type OfficePreviewPanelComponent = ComponentType let pdfPreviewPanelPromise: Promise | null = null let officePreviewPanelPromise: Promise | null = null +// `ArtifactFilePreview` re-derives `parsedBase` on every render; dedupe the +// warn per offending path so an open preview doesn't flood the log. +const warnedArtifactBasePaths = new Set() + const loadPdfPreviewPanel = () => { pdfPreviewPanelPromise ??= import('@renderer/components/ArtifactPreview/pdf/PdfPreviewPanel') .then((module) => module.default) @@ -313,10 +317,18 @@ export function ArtifactFilePreview({ // Image: binary but renderable via ``; bypass isText / size gating. if (isImageFile(filePath)) { + const base = joinPath(workspacePath, filePath) + const parsedBase = FilePathSchema.safeParse(base) + if (!parsedBase.success && !warnedArtifactBasePaths.has(base)) { + warnedArtifactBasePaths.add(base) + logger.warn('ArtifactPane: non-absolute HTML base path, relative resources may not resolve', { + path: base + }) + } return ( ) @@ -396,12 +408,20 @@ export function ArtifactFilePreview({ } if (isHtmlFile(filePath)) { + const htmlBasePath = joinPath(workspacePath, filePath) + const parsedBase = FilePathSchema.safeParse(htmlBasePath) + if (!parsedBase.success && !warnedArtifactBasePaths.has(htmlBasePath)) { + warnedArtifactBasePaths.add(htmlBasePath) + logger.warn('ArtifactPane: non-absolute HTML base path, relative resources may not resolve', { + path: htmlBasePath + }) + } return ( ) } diff --git a/src/renderer/components/chat/panes/useArtifactFileTreeModel.ts b/src/renderer/components/chat/panes/useArtifactFileTreeModel.ts index 57abf706dae..ed6c93dd048 100644 --- a/src/renderer/components/chat/panes/useArtifactFileTreeModel.ts +++ b/src/renderer/components/chat/panes/useArtifactFileTreeModel.ts @@ -2,7 +2,7 @@ import { loggerService } from '@logger' import { type FileTreeNode } from '@renderer/components/FileTree' import { useDirectoryTree } from '@renderer/hooks/useDirectoryTree' import { joinPath } from '@renderer/utils/path' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import type { CreateTreeIpcResult, DirectoryTreeOptions, @@ -248,7 +248,7 @@ function useArtifactFileSearch(workspacePath: string | undefined, searchKeyword: const timeout = setTimeout(() => { void (async () => { try { - const entries = await window.api.file.listDirectoryEntries(workspacePath as FilePath, { + const entries = await window.api.file.listDirectoryEntries(FilePathSchema.parse(workspacePath), { recursive: true, maxDepth: 0, includeHidden: false, @@ -366,7 +366,7 @@ function useLazyArtifactFileTree({ try { // One round trip that classifies each entry — avoids an `isDirectory` // IPC call per entry (was N+1 round trips per expanded folder). - const entries = await window.api.file.listDirectoryEntries(dirPath as FilePath, { + const entries = await window.api.file.listDirectoryEntries(FilePathSchema.parse(dirPath), { recursive: false, includeHidden: false, includeFiles: true, diff --git a/src/renderer/components/composer/tokenView/fileTokenPresentation.tsx b/src/renderer/components/composer/tokenView/fileTokenPresentation.tsx index 8abca1c3f1d..8ccb7ccab47 100644 --- a/src/renderer/components/composer/tokenView/fileTokenPresentation.tsx +++ b/src/renderer/components/composer/tokenView/fileTokenPresentation.tsx @@ -1,10 +1,13 @@ +import { loggerService } from '@logger' import { FILE_TYPE } from '@renderer/types/file' import type { ComposerAttachment } from '@renderer/utils/message/composerAttachment' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { toSafeFileUrl } from '@shared/utils/file' import { File, FileCode2, FileImage, FileJson, FileSpreadsheet, FileText, FileType2, Presentation } from 'lucide-react' import type { ComponentType, ReactNode } from 'react' +const logger = loggerService.withContext('fileTokenPresentation') + const fileTokenIconClassName = 'size-3 shrink-0 text-current' const fileTokenContainerClassName = 'border-border bg-background hover:bg-accent' @@ -112,7 +115,12 @@ function getFileExtensionLabel(file: ComposerAttachment | undefined, fallbackLab function getFilePreviewUrl(file: ComposerAttachment | undefined) { if (!file?.path || file.type !== FILE_TYPE.IMAGE) return undefined - return toSafeFileUrl(file.path as FilePath, file.ext || null) + const parsedPath = FilePathSchema.safeParse(file.path) + if (!parsedPath.success) { + logger.warn('getFilePreviewUrl: non-canonical/invalid attachment path', { path: file.path }) + return undefined + } + return toSafeFileUrl(parsedPath.data, file.ext || null) } function getFileTokenVariant(file: ComposerAttachment | undefined, fallbackLabel: string): FileTokenVariant { diff --git a/src/renderer/hooks/useFileSize.ts b/src/renderer/hooks/useFileSize.ts index 681b007997f..f5d49a56b22 100644 --- a/src/renderer/hooks/useFileSize.ts +++ b/src/renderer/hooks/useFileSize.ts @@ -1,6 +1,6 @@ import { loggerService } from '@logger' import { joinPath } from '@renderer/utils/path' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { createFilePathHandle } from '@shared/utils/file' import { useEffect, useState } from 'react' @@ -26,7 +26,7 @@ export function useFileSize( void (async () => { try { - const metadata = await window.api.file.getMetadata(createFilePathHandle(absPath as FilePath)) + const metadata = await window.api.file.getMetadata(createFilePathHandle(FilePathSchema.parse(absPath))) if (!cancelled) setState({ status: 'ok', size: metadata.size }) } catch (err) { if (cancelled) return diff --git a/src/renderer/pages/files/FilesPage.tsx b/src/renderer/pages/files/FilesPage.tsx index 58d200fcb6e..efb8353925f 100644 --- a/src/renderer/pages/files/FilesPage.tsx +++ b/src/renderer/pages/files/FilesPage.tsx @@ -21,7 +21,8 @@ import { safeOpen } from '@renderer/utils/file/safeOpen' import { isMac } from '@renderer/utils/platform' import type { FileEntry, FileEntryId } from '@shared/data/types/file' import type { OutputFor } from '@shared/ipc/types' -import type { FilePath, FileType } from '@shared/types/file' +import type { FileType } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { createFileEntryHandle, getFileTypeByExt, toSafeFileUrl } from '@shared/utils/file' import { MoreHorizontal, Upload } from 'lucide-react' import { memo, useCallback, useDeferredValue, useEffect, useMemo, useRef, useState } from 'react' @@ -123,7 +124,7 @@ async function requestBatchedInternalEntryCreates(paths: readonly string[]): Pro const results = await Promise.all( chunks.map((chunk) => ipcApi.request('file.batch_create_internal_entries', { - items: chunk.map((path) => ({ source: 'path' as const, path })) + items: chunk.map((path) => ({ source: 'path' as const, path: FilePathSchema.parse(path) })) }) ) ) @@ -195,7 +196,7 @@ function toFileItem( ...base, ...originFields, type, - previewUrl: physicalPath ? toSafeFileUrl(physicalPath as FilePath, entry.ext) : undefined + previewUrl: physicalPath ? toSafeFileUrl(physicalPath, entry.ext) : undefined } } diff --git a/src/renderer/pages/paintings/components/__tests__/PaintingComposerSwitch.test.tsx b/src/renderer/pages/paintings/components/__tests__/PaintingComposerSwitch.test.tsx index 711146a7698..7c424946ebd 100644 --- a/src/renderer/pages/paintings/components/__tests__/PaintingComposerSwitch.test.tsx +++ b/src/renderer/pages/paintings/components/__tests__/PaintingComposerSwitch.test.tsx @@ -1,5 +1,6 @@ import type { ComposerSurfaceProps } from '@renderer/components/composer/ComposerSurface' import type { FileEntry } from '@shared/data/types/file' +import type { FilePath } from '@shared/types/file' import { render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -58,8 +59,8 @@ describe('PaintingComposer painting switch', () => { ...window.api, file: { ...window.api.file, - getPhysicalPath: vi.fn(async ({ id }: { id: string }) => `/p/${id}.png`), - createInternalEntry: vi.fn(async ({ path }: { path: string }) => makeEntry(path)) + getPhysicalPath: vi.fn(async ({ id }: { id: string }) => `/p/${id}.png` as FilePath), + createInternalEntry: vi.fn(async ({ path }: { path: FilePath }) => makeEntry(path)) } } as typeof window.api }) diff --git a/src/renderer/pages/paintings/hooks/__tests__/usePaintingComposerInputFiles.test.ts b/src/renderer/pages/paintings/hooks/__tests__/usePaintingComposerInputFiles.test.ts index f2a9ac0ed04..9b1e8fe7b73 100644 --- a/src/renderer/pages/paintings/hooks/__tests__/usePaintingComposerInputFiles.test.ts +++ b/src/renderer/pages/paintings/hooks/__tests__/usePaintingComposerInputFiles.test.ts @@ -1,6 +1,7 @@ import { toast } from '@renderer/services/toast' import type { ComposerAttachment } from '@renderer/utils/message/composerAttachment' import type { FileEntry } from '@shared/data/types/file' +import type { FilePath } from '@shared/types/file' import { act, renderHook, waitFor } from '@testing-library/react' import { useState } from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -22,8 +23,8 @@ const makeAttachment = (sourceId: string, path: string): ComposerAttachment => ( describe('usePaintingComposerInputFiles', () => { beforeEach(() => { - const getPhysicalPath = vi.fn(async (params: { id: string }) => `/p/${params.id}.png`) - const createInternalEntry = vi.fn(async (params: { path: string }) => + const getPhysicalPath = vi.fn(async (params: { id: string }) => `/p/${params.id}.png` as FilePath) + const createInternalEntry = vi.fn(async (params: { path: FilePath }) => makeEntry(params.path.includes('new') ? 'fe-new' : 'fe-x') ) window.api = { diff --git a/src/renderer/pages/paintings/hooks/usePaintingComposerInputFiles.ts b/src/renderer/pages/paintings/hooks/usePaintingComposerInputFiles.ts index 5f1f0dc5cb4..f5df6d4ba0f 100644 --- a/src/renderer/pages/paintings/hooks/usePaintingComposerInputFiles.ts +++ b/src/renderer/pages/paintings/hooks/usePaintingComposerInputFiles.ts @@ -3,7 +3,7 @@ import { toast } from '@renderer/services/toast' import type { ComposerAttachment } from '@renderer/utils/message/composerAttachment' import { createComposerFileTokenSourceId } from '@renderer/utils/message/composerFileTokenSource' import type { FileEntry } from '@shared/data/types/file' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { getFileTypeByExt } from '@shared/utils/file' import { type Dispatch, type SetStateAction, useEffect, useRef } from 'react' import { useTranslation } from 'react-i18next' @@ -121,7 +121,10 @@ export function usePaintingComposerInputFiles({ paintingId, inputFiles, files, s continue } try { - const entry = await window.api.file.createInternalEntry({ source: 'path', path: file.path as FilePath }) + const entry = await window.api.file.createInternalEntry({ + source: 'path', + path: FilePathSchema.parse(file.path) + }) cache.set(file.fileTokenSourceId, entry) entries.push(entry) } catch (error) { diff --git a/src/renderer/pages/paintings/utils/__tests__/paintingFileUrl.test.ts b/src/renderer/pages/paintings/utils/__tests__/paintingFileUrl.test.ts index 081475dd5fe..b7e61ac0a10 100644 --- a/src/renderer/pages/paintings/utils/__tests__/paintingFileUrl.test.ts +++ b/src/renderer/pages/paintings/utils/__tests__/paintingFileUrl.test.ts @@ -1,8 +1,22 @@ -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + warn: vi.fn() +})) + +vi.mock('@logger', () => ({ + loggerService: { + withContext: () => ({ warn: mocks.warn }) + } +})) import { getPaintingFileUrl } from '../paintingFileUrl' describe('getPaintingFileUrl', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + it('returns undefined when no physical path is available', () => { expect(getPaintingFileUrl({ path: '', ext: '.png' })).toBeUndefined() }) @@ -16,6 +30,15 @@ describe('getPaintingFileUrl', () => { ).toBe('file:///resolved%20output/paint%20result.png') }) + it('returns undefined without throwing when the path is not absolute, and logs a warning', () => { + expect(() => getPaintingFileUrl({ path: 'relative/legacy.png', ext: '.png' })).not.toThrow() + expect(getPaintingFileUrl({ path: 'relative/legacy.png', ext: '.png' })).toBeUndefined() + expect(mocks.warn).toHaveBeenCalledWith( + 'getPaintingFileUrl: non-canonical/invalid painting path', + expect.objectContaining({ path: 'relative/legacy.png' }) + ) + }) + it('keeps shared file-url safety behavior', () => { expect( getPaintingFileUrl({ diff --git a/src/renderer/pages/paintings/utils/paintingFileUrl.ts b/src/renderer/pages/paintings/utils/paintingFileUrl.ts index 811ede79ac9..c0a935dc960 100644 --- a/src/renderer/pages/paintings/utils/paintingFileUrl.ts +++ b/src/renderer/pages/paintings/utils/paintingFileUrl.ts @@ -1,7 +1,10 @@ +import { loggerService } from '@logger' import type { FileMetadata } from '@renderer/types/file' -import type { FilePath, FileUrlString } from '@shared/types/file' +import { FilePathSchema, type FileUrlString } from '@shared/types/file' import { toSafeFileUrl } from '@shared/utils/file' +const logger = loggerService.withContext('paintingFileUrl') + type PaintingFileUrlSource = Pick /** @@ -11,5 +14,10 @@ type PaintingFileUrlSource = Pick */ export function getPaintingFileUrl(file: PaintingFileUrlSource): FileUrlString | undefined { if (!file.path) return undefined - return toSafeFileUrl(file.path as FilePath, file.ext || null) + const parsedPath = FilePathSchema.safeParse(file.path) + if (!parsedPath.success) { + logger.warn('getPaintingFileUrl: non-canonical/invalid painting path', { path: file.path }) + return undefined + } + return toSafeFileUrl(parsedPath.data, file.ext || null) } diff --git a/src/renderer/pages/translate/TranslatePage.tsx b/src/renderer/pages/translate/TranslatePage.tsx index f81abf16296..79f93bd26f7 100644 --- a/src/renderer/pages/translate/TranslatePage.tsx +++ b/src/renderer/pages/translate/TranslatePage.tsx @@ -43,7 +43,7 @@ import { type UniqueModelId } from '@shared/data/types/model' import type { TranslateHistory } from '@shared/data/types/translate' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { MB } from '@shared/utils/constants' import { createFilePathHandle } from '@shared/utils/file' import { documentExts, imageExts, textExts } from '@shared/utils/file' @@ -501,7 +501,7 @@ const TranslatePage: FC = () => { try { const snapshot = await ipcApi.request('file_processing.start_job', { feature: 'image_to_text', - file: createFilePathHandle(file.path as FilePath) + file: createFilePathHandle(FilePathSchema.parse(file.path)) }) jobId = snapshot.id } catch (error) { diff --git a/src/renderer/utils/file/__tests__/buildFileParts.test.ts b/src/renderer/utils/file/__tests__/buildFileParts.test.ts index 171fcf1b397..aebdeb90247 100644 --- a/src/renderer/utils/file/__tests__/buildFileParts.test.ts +++ b/src/renderer/utils/file/__tests__/buildFileParts.test.ts @@ -67,4 +67,14 @@ describe('buildFilePartsForAttachments', () => { expect(part.mediaType).toBe('application/pdf') expect(part.url).toBe('file:///p/fe-3.pdf') }) + + it('rejects the whole batch when an attachment path is non-absolute (send flow surfaces it)', async () => { + // A non-absolute path fails `FilePathSchema.parse`, which rejects the + // Promise.all batch rather than silently dropping the attachment — the + // caller's send-flow try/catch turns that into a toast + keep-editing. + const goodAttachment = attachment() + const badAttachment = attachment({ path: 'not-absolute.png', name: 'bad.png' }) + + await expect(buildFilePartsForAttachments([goodAttachment, badAttachment])).rejects.toThrow(/absolute/i) + }) }) diff --git a/src/renderer/utils/file/buildFileParts.ts b/src/renderer/utils/file/buildFileParts.ts index a3e81cceb91..2894ff34ca7 100644 --- a/src/renderer/utils/file/buildFileParts.ts +++ b/src/renderer/utils/file/buildFileParts.ts @@ -14,7 +14,7 @@ import type { ComposerAttachment } from '@renderer/utils/message/composerAttachment' import type { FileUIPart } from '@shared/data/types/message' import { withCherryMeta } from '@shared/data/types/uiParts' -import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' import { createFilePathHandle } from '@shared/utils/file' /** @@ -22,11 +22,20 @@ import { createFilePathHandle } from '@shared/utils/file' * FileEntry (Cherry copies the bytes into its own storage) and return a * `FileUIPart` that carries the new `fileEntryId` plus a `file://` URL * pointing at the freshly-copied physical file. + * + * `attachment.path` is validated through `FilePathSchema.parse` (not an `as` + * cast). Any failure — a non-absolute / malformed path, or a rejected + * `createInternalEntry` — rejects the whole batch, so the caller's send-flow + * try/catch surfaces it (toast + keep editing) rather than silently dropping a + * file the user attached. */ export async function buildFilePartsForAttachments(attachments: ComposerAttachment[]): Promise { return Promise.all( attachments.map(async (attachment) => { - const entry = await window.api.file.createInternalEntry({ source: 'path', path: attachment.path as FilePath }) + const entry = await window.api.file.createInternalEntry({ + source: 'path', + path: FilePathSchema.parse(attachment.path) + }) const physicalPath = await window.api.file.getPhysicalPath({ id: entry.id }) const metadata = await window.api.file.getMetadata(createFilePathHandle(physicalPath)) const basePart: FileUIPart = { diff --git a/src/renderer/utils/knowledgeFileEntry.ts b/src/renderer/utils/knowledgeFileEntry.ts index dfb1063d2f8..1d072377c6d 100644 --- a/src/renderer/utils/knowledgeFileEntry.ts +++ b/src/renderer/utils/knowledgeFileEntry.ts @@ -1,9 +1,10 @@ import type { FileMetadata } from '@renderer/types/file' -import { AbsolutePathSchema } from '@shared/data/types/file' +import type { FilePath } from '@shared/types/file' +import { FilePathSchema } from '@shared/types/file' export interface KnowledgeFileItemData { source: string - path: string + path: FilePath } export const resolveKnowledgeFileData = async ( @@ -16,13 +17,14 @@ export const resolveKnowledgeFileData = async ( throw new Error(`Failed to resolve a local path for "${displayName}"`) } - if (!AbsolutePathSchema.safeParse(source).success) { + const result = FilePathSchema.safeParse(source) + if (!result.success) { throw new Error(`Failed to resolve an absolute local path for "${displayName}"`) } return { source, - path: source + path: result.data } } diff --git a/src/renderer/utils/path.ts b/src/renderer/utils/path.ts index eed08c0404e..508cf822905 100644 --- a/src/renderer/utils/path.ts +++ b/src/renderer/utils/path.ts @@ -3,6 +3,7 @@ * and a trailing separator on `base`. Leading separators on `rel` are stripped so the result stays * anchored to `base`. */ +// TOOD: Use FilePath as signature export const joinPath = (base: string, rel: string): string => { const trimmed = rel.replace(/^[/\\]+/, '') if (!base) return trimmed diff --git a/src/shared/data/types/file.ts b/src/shared/data/types/file.ts index 9249f0c40b5..ddfc5b80210 100644 --- a/src/shared/data/types/file.ts +++ b/src/shared/data/types/file.ts @@ -108,8 +108,9 @@ * unmanaged `@main/utils/file/fs.remove(path)` separately. */ -import { type FilePath, SafeExtSchema } from '@shared/types/file' -import { canonicalizeAbsolutePath } from '@shared/utils/file' +import type { FilePath } from '@shared/types/file' +import { FilePathSchema, SafeExtSchema } from '@shared/types/file' +import { CanonicalFilePathSchema } from '@shared/utils/file' import * as z from 'zod' import { MessageIdSchema } from './message' @@ -162,87 +163,6 @@ export type FileEntryId = z.infer export const FileEntryOriginSchema = z.enum(['internal', 'external']) export type FileEntryOrigin = z.infer -// ─── Absolute Path ─── - -/** - * Absolute filesystem path (Unix or Windows). Rejects `file://` URLs — use a - * dedicated URL schema if needed. - * - * **Storage invariant for `externalPath`**: values persisted in - * `file_entry.externalPath` must be the output of - * `canonicalizeExternalPath()` — currently `path.resolve` + Unicode NFC + - * trailing-separator strip. Zod cannot enforce this shape - * at the schema level; `ensureExternalEntry` and `fileEntryService.findByExternalPath` - * are the application-layer enforcement points. See `pathResolver.ts` for - * the full contract, including deliberately deferred normalization steps - * (case-insensitive FS dedupe, symlink target resolution). - */ -export const AbsolutePathSchema = z - .string() - .min(1) - .refine((s) => !s.includes('\0'), 'externalPath must not contain null bytes') - .refine((s) => s.startsWith('/') || /^[A-Za-z]:\\/.test(s), 'externalPath must be an absolute filesystem path') - -// ─── Canonical External Path (TS phantom brand) ─── - -/** - * A `string` already processed through `canonicalizeExternalPath`. - * - * This is a **TypeScript-only phantom brand** (zero runtime cost, zero wire - * cost) that acts as a compile-time guard for every DB read/write surface on - * `externalPath`: any query entry point that filters by `externalPath` MUST - * narrow its input to this type, which forces callers through - * `canonicalizeExternalPath()` instead of accepting a raw user path. - * - * ## Why a brand and not runtime validation - * - * The correctness invariant — "the string equals `canonicalizeExternalPath(x)` - * for some `x`" — cannot be verified at runtime without re-running - * canonicalization, which would defeat the purpose. The brand expresses - * "this value was produced by the authorized factory" structurally, so the - * type system (not runtime checks) enforces the contract. - * - * ## Authorized construction - * - * - **Production code**: only `canonicalizeExternalPath()` in - * `src/main/services/file/utils/pathResolver.ts` may produce values of this type. - * Other production code importing `CanonicalExternalPath` MUST receive it - * from that function (directly or transitively) — never via `as` cast. - * - **Tests and fixtures**: may cast known-canonical string literals with - * `'/abs/path' as CanonicalExternalPath` for readability. - * - **DB rows**: the `externalPath` column is typed as `string | null` in - * Drizzle (SQLite has no brand concept); upcasting into - * `CanonicalExternalPath` at the service boundary is acceptable because - * writes on that column already go through the canonicalization path. - */ -// String-literal brand rather than a `unique symbol`: a `unique symbol` brand -// (named or inline) is inaccessible when TS emits a large inferred aggregate -// type that transitively embeds it (e.g. preload's -// `export type WindowApiType = typeof api`, via `FileEntry.externalPath`), -// triggering TS2527/TS4023. A literal-keyed brand is fully nameable across -// modules, so it dodges that while keeping the nominal identity — a plain -// `string` still lacks the property, so the value can only be produced by the -// canonicalization path or an explicit `as` cast, exactly as documented above. -export type CanonicalExternalPath = string & { readonly __brand: 'CanonicalExternalPath' } - -/** - * Intersection brand carried by the `externalPath` field on the FileEntry - * BO: a string that is both **canonical** (provenance: passed through - * `canonicalizeAbsolutePath` / `canonicalizeExternalPath`) and **satisfies - * the `FilePath` template-literal shape** (so it can flow into any - * `@main/utils/file/*` API without a cast). - * - * Round 2 S5: the schema's `externalPath` field used to be plain - * `AbsolutePathSchema` (inferred as `string`), forcing five production - * sites to `as FilePath`-cast at every read. The schema now `refine`s - * against `canonicalizeAbsolutePath` (real runtime check; rejects any - * non-canonical input at parse time) and then `transform`s the result - * into this intersection — so consumers reading `entry.externalPath` - * get a value typed exactly as they need it, with the canonical - * provenance proven at the schema boundary. - */ -export type CanonicalFilePath = FilePath & CanonicalExternalPath - // ─── FileEntry Schema (discriminated union on origin, branded) ─── // // ## DB row vs Business Object @@ -329,29 +249,20 @@ export const ExternalEntrySchema = z.strictObject({ ...CommonEntryFields, origin: z.literal('external'), /** - * Absolute filesystem path to the user-provided file. The schema runs a - * **real** `canonicalize` equivalence check (not just a shape match): the - * input must equal `canonicalizeAbsolutePath(input)`, otherwise parse - * rejects. Combined with the `.transform` below, this means any value the - * BO ever exposes is provably canonical AND carries the `FilePath` shape, - * eliminating the five `as FilePath` casts that used to sit at every read - * site (rename.ts, lifecycle.ts, danglingCache.ts, …). + * Absolute filesystem path to the user-provided file, as a + * `CanonicalFilePath` — the byte-faithful lexical form produced by + * `canonicalizeFilePath` (segment-resolve + trailing-strip + drive-upcase, + * NOT Unicode-normalized). Validated by `CanonicalFilePathSchema`, which is + * assert-only: this byte-faithful form is guaranteed at WRITE time + * (external-entry insert / rename), and on READ a stored value not already in + * that lexical form (a raw `./` / `..` / trailing-slash path) is REJECTED + * (surfaced via `rowToFileEntrySafe`'s warn-skip), never silently repaired — + * a byte-faithful path, including one carrying NFD Unicode, is accepted + * as-is. Rejecting rather than repairing keeps the lookup/dedup key stable, + * so a re-canonicalization migration is the only sanctioned way to fix + * historically non-canonical rows. */ - externalPath: AbsolutePathSchema.refine((s) => { - // canonicalizeAbsolutePath throws on structural failures (non-absolute, - // contains \0) — both already surfaced by `AbsolutePathSchema`'s own - // refines, but Zod does not short-circuit on prior refine failure, so we - // must absorb the throw here. Failure → return false → schema rejects - // with the canonicalization message (and the prior issue is also - // reported, giving the caller the full picture). - try { - return s === canonicalizeAbsolutePath(s) - } catch { - return false - } - }, 'externalPath must be canonicalized via canonicalizeExternalPath() before persistence').transform( - (s): CanonicalFilePath => s as CanonicalFilePath - ) + externalPath: CanonicalFilePathSchema }) /** @@ -436,7 +347,7 @@ export const FileEntryHandleSchema = z.strictObject({ export const FilePathHandleSchema = z.strictObject({ kind: z.literal('path'), - path: AbsolutePathSchema + path: FilePathSchema }) export const FileHandleSchema = z.discriminatedUnion('kind', [FileEntryHandleSchema, FilePathHandleSchema]) diff --git a/src/shared/data/types/fileProcessing.ts b/src/shared/data/types/fileProcessing.ts index 3aea2d2f85c..844d676f813 100644 --- a/src/shared/data/types/fileProcessing.ts +++ b/src/shared/data/types/fileProcessing.ts @@ -1,7 +1,7 @@ +import { FilePathSchema } from '@shared/types/file' import * as z from 'zod' import { FILE_PROCESSOR_IDS } from '../preference/preferenceTypes' -import { AbsolutePathSchema } from './file' export const FileProcessingTextArtifactSchema = z .object({ @@ -15,7 +15,7 @@ export const FileProcessingFileArtifactSchema = z .object({ kind: z.literal('file'), format: z.literal('markdown'), - path: AbsolutePathSchema + path: FilePathSchema }) .strict() @@ -25,7 +25,7 @@ export const FileProcessingArtifactSchema = z.discriminatedUnion('kind', [ ]) export type FileProcessingArtifact = z.infer -export const FileProcessingOutputTargetSchema = z.object({ kind: z.literal('path'), path: AbsolutePathSchema }).strict() +export const FileProcessingOutputTargetSchema = z.object({ kind: z.literal('path'), path: FilePathSchema }).strict() export type FileProcessingOutputTarget = z.infer export const FileProcessingJobOutputSchema = z diff --git a/src/shared/data/types/knowledge.ts b/src/shared/data/types/knowledge.ts index 372a60c55e4..90a033f1f34 100644 --- a/src/shared/data/types/knowledge.ts +++ b/src/shared/data/types/knowledge.ts @@ -1,6 +1,6 @@ +import { FilePathSchema } from '@shared/types/file' import * as z from 'zod' -import { AbsolutePathSchema } from './file' import { GroupIdSchema } from './group' /** @@ -637,11 +637,11 @@ export const CreateKnowledgeItemSchema = z.discriminatedUnion('type', [ export type CreateKnowledgeItemDto = z.infer const RuntimeFileItemDataSchema = KnowledgeItemSharedSchema.extend({ - path: AbsolutePathSchema.describe('Absolute source path selected by the user before Knowledge copies it.'), + path: FilePathSchema.describe('Absolute source path selected by the user before Knowledge copies it.'), // Restore-only: absolute path to an already-produced processor artifact (e.g. MinerU // Markdown) in the source base. When present, Knowledge copies it in alongside the // source file and indexes from it directly, skipping the file processor. - indexedPath: AbsolutePathSchema.optional().describe( + indexedPath: FilePathSchema.optional().describe( 'Absolute path to an already-processed artifact to copy in and index from, skipping the file processor.' ) }) @@ -652,7 +652,7 @@ const RuntimeUrlItemDataSchema = KnowledgeItemSharedSchema.extend({ // When present, Knowledge copies it in and pins the item to it so the first index // reads the snapshot offline instead of re-fetching the (possibly changed or dead) // live page. Omitted by a normal add, which captures lazily on first index. - snapshotPath: AbsolutePathSchema.optional().describe( + snapshotPath: FilePathSchema.optional().describe( 'Absolute path to a captured URL snapshot markdown to copy in, skipping the live re-fetch.' ) }) diff --git a/src/shared/ipc/schemas/file.ts b/src/shared/ipc/schemas/file.ts index 2a37f547096..d34ecc8235c 100644 --- a/src/shared/ipc/schemas/file.ts +++ b/src/shared/ipc/schemas/file.ts @@ -1,12 +1,6 @@ -import { - AbsolutePathSchema, - DanglingStateSchema, - FileEntryIdSchema, - FileEntrySchema, - SafeNameSchema -} from '@shared/data/types/file' +import { DanglingStateSchema, FileEntryIdSchema, FileEntrySchema, SafeNameSchema } from '@shared/data/types/file' import { FileHandleSchema } from '@shared/data/types/file' -import { PhysicalFileMetadataSchema, SafeExtSchema } from '@shared/types/file' +import { FilePathSchema, PhysicalFileMetadataSchema, SafeExtSchema } from '@shared/types/file' import * as z from 'zod' import { defineRoute } from '../define' @@ -42,7 +36,7 @@ const batchCreateResultSchema = z.strictObject({ // future drift; refactor them to share one source of truth before migrating the // remaining File IPC surface. const createInternalEntryInputSchema = z.discriminatedUnion('source', [ - z.strictObject({ source: z.literal('path'), path: AbsolutePathSchema }), + z.strictObject({ source: z.literal('path'), path: FilePathSchema }), z.strictObject({ source: z.literal('url'), url: z.url() }), z.strictObject({ source: z.literal('base64'), data: z.string().min(1), name: SafeNameSchema.optional() }), z.strictObject({ @@ -70,7 +64,7 @@ export const fileRequestSchemas = { }), 'file.batch_get_physical_paths': defineRoute({ input: fileEntryIdsInputSchema, - output: z.record(z.string(), AbsolutePathSchema.nullable()) + output: z.record(z.string(), FilePathSchema.nullable()) }), 'file.batch_get_dangling_states': defineRoute({ input: fileEntryIdsInputSchema, diff --git a/src/shared/types/file/__tests__/FilePathSchema.test.ts b/src/shared/types/file/__tests__/FilePathSchema.test.ts new file mode 100644 index 00000000000..022eae66044 --- /dev/null +++ b/src/shared/types/file/__tests__/FilePathSchema.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' + +import { FilePathSchema } from '../common' + +describe('FilePathSchema', () => { + // FilePathSchema validates absolute-path SHAPE only and does NOT canonicalize: + // `parse(x)` returns `x` byte-for-byte. Canonicalization (byte-faithful + // lexical cleanup: segment-resolve + trailing-strip + drive-letter upcase, + // NOT Unicode-normalized) is a separate concern owned by `canonicalizeFilePath` + // / `CanonicalFilePathSchema` — its behavior is pinned in + // `@shared/utils/file/__tests__/canonicalize.test.ts`. + + it('returns a POSIX absolute path unchanged', () => { + expect(FilePathSchema.parse('/Users/me/doc.pdf')).toBe('/Users/me/doc.pdf') + }) + + it('returns a Windows backslash absolute path unchanged', () => { + expect(FilePathSchema.parse('C:\\Users\\me\\doc.pdf')).toBe('C:\\Users\\me\\doc.pdf') + }) + + it('accepts a Windows forward-slash absolute path and returns it unchanged (no backslash conversion)', () => { + expect(FilePathSchema.parse('C:/Users/me/doc.pdf')).toBe('C:/Users/me/doc.pdf') + }) + + it('does NOT NFC-normalize decomposed (NFD) input — returns it unchanged', () => { + const nfd = '/Users/me/cafe\u0301.txt' // "cafe\u0301" as e + U+0301 combining acute (NFD) + expect(FilePathSchema.parse(nfd)).toBe(nfd) + }) + + it('does NOT strip a trailing separator — returns it unchanged', () => { + expect(FilePathSchema.parse('/foo/bar/')).toBe('/foo/bar/') + }) + + it('does NOT resolve . and .. segments — returns them unchanged', () => { + expect(FilePathSchema.parse('/foo/./baz/../bar')).toBe('/foo/./baz/../bar') + }) + + it('rejects a relative path', () => { + expect(FilePathSchema.safeParse('foo/bar').success).toBe(false) + expect(FilePathSchema.safeParse('./foo').success).toBe(false) + }) + + it('rejects a file:// URL', () => { + expect(FilePathSchema.safeParse('file:///Users/me/doc.pdf').success).toBe(false) + }) + + it('rejects an empty string', () => { + expect(FilePathSchema.safeParse('').success).toBe(false) + }) + + it('rejects a null byte', () => { + expect(FilePathSchema.safeParse('/foo/\0bar').success).toBe(false) + }) +}) diff --git a/src/shared/types/file/common.ts b/src/shared/types/file/common.ts index 9f95b4953c0..d4852a719d2 100644 --- a/src/shared/types/file/common.ts +++ b/src/shared/types/file/common.ts @@ -29,13 +29,46 @@ export type FileType = z.infer // ─── Content Source Types ─── /** - * Local filesystem path (absolute Unix or Windows). + * Absolute filesystem path validated by `FilePathSchema`. Validates the path + * SHAPE only — absolute form, no null bytes — and does NOT canonicalize: + * `FilePathSchema.parse(x)` returns `x` unchanged (the path exactly as given). * - * Runtime validation required — the template-literal pattern only provides - * type-level hints. Rejects `file://` URLs; use a dedicated URL type (or plain - * `string`) when a consumer needs to accept URLs. + * The canonical form of a path (byte-faithful lexical resolve: segment-resolve + * + trailing-separator strip + drive-letter upcase, NOT Unicode-normalized) is + * a distinct `CanonicalFilePath` produced by `canonicalizeFilePath()` + * (`@shared/utils/file/canonicalize`); it is applied explicitly at the + * external-path persistence / lookup boundary, not on every `FilePath` parse. + * + * The `z.brand` is a phantom brand — zero runtime cost, dropped on IPC + * serialization; receivers re-assert via `FilePathSchema.parse()` at the + * trusted boundary. Construction: + * - Production: `FilePathSchema.parse(raw)` / `.safeParse(raw)` + * - Tests / fixtures: `'…' as FilePath` for readability + * + * Accepts POSIX (`/…`) and Windows (`X:\…` or `X:/…`) absolute forms. + * Rejects `file://` URLs. + * + * Known limitation (pre-existing): UNC paths (`\\server\share`) are rejected + * by the absolute-shape refine above — they match neither the POSIX `/` form + * nor the Windows `X:[/\\]` drive-letter form. + */ +export const FilePathSchema = z + .string() + .min(1) + .refine((s) => !s.includes('\0'), 'must not contain null bytes') + .refine((s) => s.startsWith('/') || /^[A-Za-z]:[/\\]/.test(s), 'must be an absolute filesystem path') + .brand<'FilePath'>() + +export type FilePath = z.infer + +/** + * `CanonicalFilePath` — a `FilePath` additionally proven canonical — is defined + * in `@shared/utils/file/canonicalize` (co-located with its schema and the + * `canonicalizeFilePath()` factory, which is the only place that can build both + * the value and its brand). It is not re-exported here. */ -export type FilePath = `/${string}` | `${string}:\\${string}` + +// TODO: Add schema for them export type Base64String = `data:${string};base64,${string}` export type UrlString = `http://${string}` | `https://${string}` diff --git a/src/shared/types/file/index.ts b/src/shared/types/file/index.ts index bc245078899..ed2f855307b 100644 --- a/src/shared/types/file/index.ts +++ b/src/shared/types/file/index.ts @@ -5,6 +5,7 @@ export { FILE_TYPE, type FileContent, type FilePath, + FilePathSchema, type FileType, FileTypeSchema, type FileUrlString, diff --git a/src/shared/types/file/info.ts b/src/shared/types/file/info.ts index 778cb38c321..1321d6c0298 100644 --- a/src/shared/types/file/info.ts +++ b/src/shared/types/file/info.ts @@ -58,8 +58,7 @@ import * as z from 'zod' -import type { FilePath } from './common' -import { FileTypeSchema } from './common' +import { FilePathSchema, FileTypeSchema } from './common' /** * Zod schema for `FileInfo`. Branded so consumers cannot construct a raw @@ -74,17 +73,10 @@ import { FileTypeSchema } from './common' export const FileInfoSchema = z .strictObject({ /** - * Absolute filesystem path. The TypeScript template-literal `FilePath` - * brand is enforced only at the type level; the runtime check here is - * the same shape gate (`/`-prefixed POSIX or `X:\` Windows drive) - * plus a null-byte rejection — anything that survives is safe to feed - * to `fs` APIs. + * Absolute filesystem path. Shape-validated and branded `FilePath` by + * `FilePathSchema` — anything that survives is safe to feed to `fs` APIs. */ - path: z - .string() - .min(1) - .refine((s) => !s.includes('\0'), 'path must not contain null bytes') - .refine((s) => s.startsWith('/') || /^[A-Za-z]:\\/.test(s), 'path must be an absolute filesystem path'), + path: FilePathSchema, /** Basename without extension. */ name: z.string(), /** Extension without leading dot, or `null` for extensionless files. */ @@ -109,10 +101,8 @@ export const FileInfoSchema = z * @see {@link PhysicalFileMetadata} for per-kind rich stat (dimensions, * pageCount, etc.). * - * Inferred from `FileInfoSchema`; the schema is the source of truth. - * The runtime `path` shape check is intentionally weaker than the TS - * `FilePath` template literal (template literals can't be expressed in - * Zod) — the field is still typed as `FilePath` here for ergonomics - * everywhere it crosses an IPC boundary. + * Inferred from `FileInfoSchema`; the schema is the source of truth. `path` + * already carries the `FilePath` brand via `FilePathSchema`, so no `Omit` + * hack is needed to retype it. */ -export type FileInfo = Omit, 'path'> & { readonly path: FilePath } +export type FileInfo = z.infer diff --git a/src/shared/types/file/ipc.ts b/src/shared/types/file/ipc.ts index 4e3295492f3..7676a291596 100644 --- a/src/shared/types/file/ipc.ts +++ b/src/shared/types/file/ipc.ts @@ -118,43 +118,22 @@ export type CreateInternalEntryIpcParams = * `getMetadata`. External entries cannot be trashed, so no "restore" branch * is possible. * - * ## Canonicalization stays on the main side (by design) + * ## Canonicalization * - * `externalPath` is intentionally typed as raw `FilePath` rather than - * `CanonicalExternalPath`. The asymmetry is deliberate: + * `externalPath` is typed as `FilePath` at this IPC boundary — shape-validated + * only (absolute form, no null bytes; see `FilePathSchema`), NOT canonicalized. + * Canonicalization into the stored `CanonicalFilePath` form (byte-faithful + * lexical cleanup: resolve segments, strip trailing separator, drive-letter + * upcase — NOT Unicode-normalized) happens inside `ensureExternalEntry`, via + * `canonicalizeFilePath()` (`@shared/utils/file/canonicalize`) — not at IPC + * parse time. * - * - **Renderer has no canonicalize use case.** It never compares paths - * for dedup (the DB-level `UNIQUE(externalPath)` index does that - * after `ensureExternalEntry`), never derives a canonical projection, - * and never uses paths as join keys. Every path the renderer holds - * either flows back to main (for an IPC call) or feeds a system API - * that itself accepts arbitrary user paths. - * - **Canonicalization implementation is main-only.** - * `canonicalizeExternalPath` (`src/main/services/file/utils/pathResolver.ts`) - * depends on main-only modules (realpath / NFC / case-fold). Asking the - * renderer to canonicalize would either duplicate that logic or - * require an extra IPC hop per call — no upside for either choice. - * - **The brand is already protected by a project rule, not by JSDoc.** - * `fileEntry.ts` makes the construction discipline explicit: only the - * `canonicalizeExternalPath` factory may produce `CanonicalExternalPath`; - * production code MUST NEVER `as`-cast into the brand. Code that - * bypasses the gate violates the rule, not just an inline comment — - * PR review catches it the same way it catches any other rule break. - * - * **Why not extend the brand to the IPC boundary** (e.g. a `RawExternalPath` - * brand on the param): the renderer would have to `as`-cast `string → - * RawExternalPath` at the call site, which is itself a violation of the - * same "no production `as`-cast into brands" rule. The proposal therefore - * trades one boundary's discipline for another's, without adding actual - * enforcement; meanwhile dev / test ergonomics get worse at every call - * site. The four current main consumers (FileManager.ensureExternalEntry, - * FileManager.rename, `internal/entry/rename.ts`, `internal/entry/create.ts`) - * already canonicalize before any DB lookup, and Phase 2 consumers join - * that pattern via code review at the same sites. - * - * Skipping canonicalization silently misses entries on case-insensitive - * filesystems and after symlink resolution — which is why the gate exists - * at all. + * What stays main-only is the **disambiguation** step beyond canonicalization: + * `ensureExternalEntry` additionally runs `fs.realpath` to resolve + * case-insensitive-filesystem collisions (see `internal/entry/create.ts`), + * which depends on main-only FS APIs. Skipping that disambiguation silently + * misses entries on case-insensitive filesystems and after symlink + * resolution — which is why it stays a main-side concern. */ export type EnsureExternalEntryIpcParams = { externalPath: FilePath diff --git a/src/shared/utils/file/__tests__/canonicalize.test.ts b/src/shared/utils/file/__tests__/canonicalize.test.ts index 22b85c09a72..6ddeabbb9a8 100644 --- a/src/shared/utils/file/__tests__/canonicalize.test.ts +++ b/src/shared/utils/file/__tests__/canonicalize.test.ts @@ -1,81 +1,146 @@ /** - * Equivalence tests for `canonicalizeAbsolutePath` — the shared, pure-JS - * implementation that backs the FileEntry schema's `externalPath` refine. + * Tests for the canonical layer's public surface: `canonicalizeFilePath` (the + * branding factory), `CanonicalFilePathSchema` (assert-only validation), and + * `isCanonicalFilePath` (the predicate). The canonicalization algorithm itself + * is module-private and is exercised transitively through `canonicalizeFilePath`, + * whose runtime result is exactly the canonicalized string (the brand is a + * compile-time phantom). * - * For inputs that match the host platform, the result must equal what the - * main-side `path.resolve` + NFC + trailing-strip pipeline produces; this - * keeps the canonicalize-on-write (main) and canonicalize-on-parse (schema) - * sides in lockstep. Cross-platform cases (Windows-shaped paths processed on - * POSIX hosts and vice versa) are pinned by handcrafted expectations because - * `path.resolve` is host-aware and can't be used as the oracle there. + * For inputs that match the host platform, the factory result must equal what + * the main-side `path.resolve` + trailing-strip pipeline produces + * (byte-faithful, NO Unicode normalization); this keeps the canonicalize-on-write + * path (applied at the external-path boundary) and the schema's canonical- + * equivalence assertion in lockstep. Cross-platform cases (Windows-shaped paths + * processed on POSIX hosts and vice versa) are pinned by handcrafted + * expectations because `path.resolve` is host-aware and can't be the oracle there. */ import path from 'node:path' import { describe, expect, it } from 'vitest' -import { canonicalizeAbsolutePath } from '../canonicalize' +import { CanonicalFilePathSchema, canonicalizeFilePath, isCanonicalFilePath } from '../canonicalize' + +// Byte-distinct NFD/NFC forms of ".../qué", written with ASCII \u escapes so the +// literals stay stable across editors/formatters (a precomposed source char +// would make the byte-faithfulness assertion tautological). +const QUE_NFD = '/users/qu\u0065\u0301' // q u e + U+0301 combining acute (NFD) +const QUE_NFC = '/users/qu\u00e9' // q u e-precomposed (NFC) function nodeCanonicalize(raw: string): string { + // Byte-faithful: `path.resolve` (segment resolve) + trailing-strip only. + // No `.normalize('NFC')` — canonicalization deliberately preserves the + // exact bytes so the path reaches the real file on every filesystem. let normalized = path.resolve(raw) - normalized = normalized.normalize('NFC') if (normalized.length > 1 && (normalized.endsWith(path.sep) || normalized.endsWith('/'))) { normalized = normalized.slice(0, -1) } return normalized } -describe('canonicalizeAbsolutePath — POSIX', () => { +describe('canonicalizeFilePath — POSIX', () => { it('rejects null bytes', () => { - expect(() => canonicalizeAbsolutePath('/foo/bar\0/baz')).toThrow(/null byte/i) + expect(() => canonicalizeFilePath('/foo/bar\0/baz')).toThrow(/null byte/i) }) it('rejects non-absolute input', () => { - expect(() => canonicalizeAbsolutePath('foo/bar')).toThrow(/absolute/i) + expect(() => canonicalizeFilePath('foo/bar')).toThrow(/absolute/i) }) it('collapses `.` and `..` segments', () => { - expect(canonicalizeAbsolutePath('/foo/./bar/../baz')).toBe('/foo/baz') + expect(canonicalizeFilePath('/foo/./bar/../baz')).toBe('/foo/baz') }) it('strips trailing separator (except root)', () => { - expect(canonicalizeAbsolutePath('/foo/bar/')).toBe('/foo/bar') - expect(canonicalizeAbsolutePath('/')).toBe('/') + expect(canonicalizeFilePath('/foo/bar/')).toBe('/foo/bar') + expect(canonicalizeFilePath('/')).toBe('/') }) it('collapses repeated separators', () => { - expect(canonicalizeAbsolutePath('/foo//bar')).toBe('/foo/bar') + expect(canonicalizeFilePath('/foo//bar')).toBe('/foo/bar') }) - it('NFC-normalizes Unicode', () => { - const nfd = '/users/qué' // qu + e + combining acute - const nfc = '/users/qué' // qu + é - expect(canonicalizeAbsolutePath(nfd)).toBe(nfc) + it('does NOT Unicode-normalize — returns decomposed (NFD) input byte-faithfully unchanged', () => { + // An NFD path stays NFD so it still reaches the real file on + // normalization-sensitive filesystems (Linux ext4), never folded to NFC. + expect(QUE_NFD).not.toBe(QUE_NFC) // byte-distinct inputs + expect(canonicalizeFilePath(QUE_NFD)).toBe(QUE_NFD) }) it('matches node:path on the host platform for representative inputs', () => { if (process.platform === 'win32') return // skipped on win32 (host expects \-paths) for (const raw of ['/foo/bar', '/foo/./bar/../baz', '/foo/bar/', '/foo//bar', '/']) { - expect(canonicalizeAbsolutePath(raw)).toBe(nodeCanonicalize(raw)) + expect(canonicalizeFilePath(raw)).toBe(nodeCanonicalize(raw)) } }) }) -describe('canonicalizeAbsolutePath — Windows', () => { +describe('canonicalizeFilePath — Windows', () => { it('uppercases the drive letter and uses backslash separators', () => { - expect(canonicalizeAbsolutePath('c:\\Foo\\Bar')).toBe('C:\\Foo\\Bar') + expect(canonicalizeFilePath('c:\\Foo\\Bar')).toBe('C:\\Foo\\Bar') }) it('treats `/` and `\\` interchangeably as segment separators', () => { - expect(canonicalizeAbsolutePath('C:\\foo/bar\\baz')).toBe('C:\\foo\\bar\\baz') + expect(canonicalizeFilePath('C:\\foo/bar\\baz')).toBe('C:\\foo\\bar\\baz') }) it('collapses `.` and `..` segments', () => { - expect(canonicalizeAbsolutePath('C:\\foo\\.\\bar\\..\\baz')).toBe('C:\\foo\\baz') + expect(canonicalizeFilePath('C:\\foo\\.\\bar\\..\\baz')).toBe('C:\\foo\\baz') }) it('strips trailing separator (except drive root)', () => { - expect(canonicalizeAbsolutePath('C:\\foo\\')).toBe('C:\\foo') - expect(canonicalizeAbsolutePath('C:\\')).toBe('C:\\') + expect(canonicalizeFilePath('C:\\foo\\')).toBe('C:\\foo') + expect(canonicalizeFilePath('C:\\')).toBe('C:\\') + }) +}) + +describe('CanonicalFilePathSchema (assert-only, no repair)', () => { + // Backs the FileEntry `externalPath` read path: an already-canonical value is + // accepted unchanged; a non-canonical one is REJECTED (never silently + // repaired), so historically non-canonical rows warn-skip rather than mutate + // the lookup/dedup key. + it('accepts an already-canonical path and returns it unchanged', () => { + expect(CanonicalFilePathSchema.parse('/foo/bar')).toBe('/foo/bar') + expect(CanonicalFilePathSchema.parse('C:\\Foo\\Bar')).toBe('C:\\Foo\\Bar') + }) + + it('accepts a byte-faithful (NFD) canonical path unchanged', () => { + expect(CanonicalFilePathSchema.parse(QUE_NFD)).toBe(QUE_NFD) + }) + + it('rejects a non-canonical path (unresolved `.` / `..`)', () => { + expect(CanonicalFilePathSchema.safeParse('/foo/./bar').success).toBe(false) + expect(CanonicalFilePathSchema.safeParse('/foo/bar/../baz').success).toBe(false) + }) + + it('rejects a trailing-separator path', () => { + expect(CanonicalFilePathSchema.safeParse('/foo/bar/').success).toBe(false) + }) + + it('rejects a lowercase Windows drive letter (non-canonical)', () => { + expect(CanonicalFilePathSchema.safeParse('c:\\Foo\\Bar').success).toBe(false) + }) + + it('rejects non-absolute / null-byte input', () => { + expect(CanonicalFilePathSchema.safeParse('foo/bar').success).toBe(false) + expect(CanonicalFilePathSchema.safeParse('/foo/\0bar').success).toBe(false) + }) +}) + +describe('isCanonicalFilePath', () => { + it('is true for already-canonical paths', () => { + expect(isCanonicalFilePath('/foo/bar')).toBe(true) + expect(isCanonicalFilePath('C:\\Foo\\Bar')).toBe(true) + }) + + it('is false for non-canonical paths', () => { + expect(isCanonicalFilePath('/foo/./bar')).toBe(false) + expect(isCanonicalFilePath('/foo/bar/')).toBe(false) + expect(isCanonicalFilePath('c:\\Foo\\Bar')).toBe(false) + }) + + it('is false (does not throw) for structurally invalid input', () => { + expect(isCanonicalFilePath('foo/bar')).toBe(false) + expect(isCanonicalFilePath('/foo/\0bar')).toBe(false) }) }) diff --git a/src/shared/utils/file/__tests__/handle.test.ts b/src/shared/utils/file/__tests__/handle.test.ts index 49c9524c955..f0d96d90751 100644 --- a/src/shared/utils/file/__tests__/handle.test.ts +++ b/src/shared/utils/file/__tests__/handle.test.ts @@ -12,7 +12,7 @@ describe('createFileEntryHandle', () => { describe('createFilePathHandle — runtime validation', () => { it('accepts POSIX absolute paths', () => { - const h = createFilePathHandle('/Users/me/doc.pdf') + const h = createFilePathHandle('/Users/me/doc.pdf' as FilePath) expect(h).toEqual({ kind: 'path', path: '/Users/me/doc.pdf' }) }) @@ -53,7 +53,7 @@ describe('handle type guards', () => { }) it('isFilePathHandle narrows to the path variant', () => { - const h = createFilePathHandle('/tmp/x') + const h = createFilePathHandle('/tmp/x' as FilePath) expect(isFilePathHandle(h)).toBe(true) expect(isFileEntryHandle(h)).toBe(false) }) diff --git a/src/shared/utils/file/__tests__/url.test.ts b/src/shared/utils/file/__tests__/url.test.ts index fe7d24cb4da..b2ca4f89a76 100644 --- a/src/shared/utils/file/__tests__/url.test.ts +++ b/src/shared/utils/file/__tests__/url.test.ts @@ -154,6 +154,6 @@ describe('toSafeFileUrl', () => { // renderer would end up with `file:///payload.exe`, which `` / // `` can hand to OS file associations. expect(toSafeFileUrl('/payload.exe' as FilePath, 'exe')).toBe('file:///') - expect(toSafeFileUrl('C:\\payload.exe' as FilePath, 'exe')).toBe('file:///C:') + expect(toSafeFileUrl('C:\\payload.exe' as FilePath, 'exe')).toBe('file:///C:/') }) }) diff --git a/src/shared/utils/file/canonicalize.ts b/src/shared/utils/file/canonicalize.ts index 78c17fd2c7d..c5c63652bf9 100644 --- a/src/shared/utils/file/canonicalize.ts +++ b/src/shared/utils/file/canonicalize.ts @@ -1,22 +1,33 @@ /** * Pure-JS canonicalization for absolute filesystem paths. * - * Lives in shared (no `node:*` imports) so the FileEntry schema can `refine` - * its `externalPath` field against the same canonicalization rule the main - * process uses on write. That refine is what gives the `CanonicalFilePath` - * brand on the BO real runtime backing — any value that survives parsing IS - * canonical, not just typed as if it were. + * Lives in shared (no `node:*` imports). This module owns the whole canonical + * layer: the (module-private) canonicalization algorithm, the + * `CanonicalFilePath` brand + `CanonicalFilePathSchema`, the + * `isCanonicalFilePath` predicate, and the `canonicalizeFilePath` factory. The + * schema is defined HERE, not in `types/file/common`, because it needs both + * `FilePathSchema` and the algorithm — and only `utils → types` avoids an + * import cycle. `FilePath` (`@shared/types/file/common`) is shape-validated + * only — it does NOT canonicalize on parse. Canonicalization is applied + * explicitly via `canonicalizeFilePath`, which produces the `CanonicalFilePath` + * sub-brand, at the external-path persistence / lookup boundary. * * ## Scope (this function's contract) * - * Same rules as the main-side `canonicalizeExternalPath` (see - * `src/main/services/file/utils/pathResolver.ts`) — only the implementation - * differs (this version does not depend on `node:path`): - * * 0. Reject null bytes (`\0`). * 1. Resolve segments (`.`, `..`, repeated separators). - * 2. Unicode NFC normalize. - * 3. Strip trailing separator (except on a bare drive / POSIX root). + * 2. Strip trailing separator (except on a bare drive / POSIX root). + * 3. Windows only: uppercase the drive letter, normalize separators to `\`. + * + * This is reachability-safe **lexical** cleanup only: every step is a + * filesystem no-op for *which file the path reaches* — the cleaned string + * still resolves to the same on-disk entry on every platform. The result is + * therefore **byte-faithful**: Unicode (NFC) normalization is deliberately + * NOT performed, so a canonicalized path keeps the exact bytes the OS handed + * us and always reaches the real file even on normalization-sensitive + * filesystems (Linux ext4/btrfs), where an NFC-rewritten path would not exist + * on disk. See `docs/references/file/file-manager-architecture.md §1.2 + * "Rejected: Unicode (NFC) normalization of externalPath"`. * * The input **must already be absolute**. POSIX absolute (`/…`) and Windows * absolute (`X:\…` or `X:/…`) are both accepted; mixed-platform input is @@ -25,22 +36,80 @@ * * ## Rule-evolution discipline * - * Changing the normalization steps below desynchronizes historical rows - * (written under the old rule) from new queries (running under the new - * rule). Any such change MUST ship with a paired Drizzle migration that - * re-canonicalizes every existing `file_entry.externalPath` and re-points - * association rows whose canonical forms now collide. See - * `docs/references/file/file-manager-architecture.md §1.2 Rule evolution - * discipline`. + * The steps above are lexical only and do not depend on filesystem Unicode + * semantics, so they do NOT carry the "changing the rule desyncs historical + * rows, requiring a paired re-canonicalize migration" hazard that the removed + * NFC step did. See `docs/references/file/file-manager-architecture.md §1.2 + * "Residual normalization discipline"`. */ -export function canonicalizeAbsolutePath(raw: string): string { +import { FilePathSchema } from '@shared/types/file' +import type * as z from 'zod' + +function canonicalizeAbsolutePath(raw: string): string { if (raw.includes('\0')) { throw new Error('canonicalizeAbsolutePath: input contains null byte') } const isWindows = /^[A-Za-z]:[/\\]/.test(raw) const normalized = isWindows ? canonicalizeWindows(raw) : canonicalizePosix(raw) - return normalized.normalize('NFC') + return normalized +} + +/** + * True iff `p` is already in byte-faithful canonical form — i.e. canonicalizing + * it is a no-op. Returns `false` (rather than throwing) for structurally + * invalid input (non-absolute, null byte), so it is safe as a Zod refine + * predicate. Backs both `CanonicalFilePathSchema` and the FileEntry + * `externalPath` read-path assertion. + */ +export function isCanonicalFilePath(p: string): boolean { + try { + return p === canonicalizeAbsolutePath(p) + } catch { + return false + } +} + +/** + * Zod schema + brand for `CanonicalFilePath`, mirroring how `FilePathSchema` + * defines `FilePath` (both are `z.infer`-derived from their schema). Reuses + * `FilePathSchema` for the absolute-shape refine, then ASSERTS byte-faithful + * canonical form (via `isCanonicalFilePath`) and brands. + * + * `parse()` is assert-only, NOT repairing: it returns an already-canonical + * input unchanged and REJECTS (ZodError) a non-canonical one. This is the "no + * silent repair" contract the FileEntry `externalPath` read path depends on. + * To canonicalize a raw path, use `canonicalizeFilePath` (which repairs, then + * brands through this schema). + */ +export const CanonicalFilePathSchema = FilePathSchema.refine( + isCanonicalFilePath, + 'must be in byte-faithful canonical form (produce it via canonicalizeFilePath)' +).brand<'CanonicalFilePath'>() + +/** + * A `FilePath` additionally proven canonical — the byte-faithful, lexically + * resolved form (segment-resolve + trailing-strip + Windows drive-upcase), NOT + * Unicode-normalized. This is the form persisted in `file_entry.externalPath` + * and used as the dedup / lookup key. Inferred from `CanonicalFilePathSchema`, + * so its definition style matches `FilePath`. A `CanonicalFilePath` IS a + * `FilePath`, accepted anywhere a `FilePath` is. + */ +export type CanonicalFilePath = z.infer + +/** + * The sole sanctioned producer of `CanonicalFilePath`: canonicalize a raw + * absolute path (byte-faithful lexical resolve — segment-resolve + + * trailing-strip + Windows drive-upcase, NOT Unicode-normalized) and brand the + * result through `CanonicalFilePathSchema`. Callers needing a canonical + * lookup/persistence key (external-entry write + `findByExternalPath`) go + * through here. + * + * @throws if `input` is not absolute / contains a null byte (delegated to the + * canonicalization algorithm, before branding). + */ +export function canonicalizeFilePath(input: string): CanonicalFilePath { + return CanonicalFilePathSchema.parse(canonicalizeAbsolutePath(input)) } function canonicalizePosix(raw: string): string { @@ -63,7 +132,11 @@ function canonicalizePosix(raw: string): string { function canonicalizeWindows(raw: string): string { // Drive letter is uppercased so `C:\Foo` and `c:\Foo` canonicalize to the // same string at the byte layer — case folding the path itself is - // deliberately deferred (see pathResolver.ts JSDoc for the rationale). + // deliberately deferred: case-insensitive dedup is handled by the DB + // `lower(externalPath)` unique index plus an `fs.realpath` collision probe + // (see docs/references/file/file-manager-architecture.md §1.2 + // "Duplicate-entry detection on insert"), so no per-segment case-fold is + // needed here. const drive = raw.slice(0, 2).toUpperCase() const segments = raw.slice(3).split(/[/\\]/) const stack: string[] = [] diff --git a/src/shared/utils/file/index.ts b/src/shared/utils/file/index.ts index e1872b49adf..adeec0ef7f1 100644 --- a/src/shared/utils/file/index.ts +++ b/src/shared/utils/file/index.ts @@ -1,4 +1,9 @@ -export { canonicalizeAbsolutePath } from './canonicalize' +export { + type CanonicalFilePath, + CanonicalFilePathSchema, + canonicalizeFilePath, + isCanonicalFilePath +} from './canonicalize' export { audioExts, codeLangExts, diff --git a/src/shared/utils/file/url.ts b/src/shared/utils/file/url.ts index c1572fcec8e..cbfb63bdc3f 100644 --- a/src/shared/utils/file/url.ts +++ b/src/shared/utils/file/url.ts @@ -33,7 +33,7 @@ * access, or main-process singletons belongs in File IPC. */ -import { type FilePath, type FileUrlString, SafeExtSchema } from '@shared/types/file' +import { type FilePath, FilePathSchema, type FileUrlString, SafeExtSchema } from '@shared/types/file' // ─── Danger extension policy ─── @@ -141,17 +141,26 @@ export function isDangerExt(ext: string | null | undefined): boolean { // ─── Path formatting ─── /** - * Cross-platform dirname on a plain string — no `node:path` dependency, so it + * Cross-platform dirname on a `FilePath` — no `node:path` dependency, so it * works in renderer bundles. Treats both `/` and `\` as separators. * * `sepIdx === 0` is the POSIX-root case (`/payload.exe`): degrade to `'/'` so * the safety wrap in `toSafeFileUrl` still strips the filename. Returning the * original string here would defeat the entire danger-ext policy. + * + * The Windows-drive-root case (`C:\payload.exe`) needs the same treatment: + * slicing off just the separator would leave a bare `C:`, which + * `FilePathSchema` rejects as non-absolute (it requires the trailing + * separator to recognize a drive letter as a root). Keep the separator so + * the result stays a valid, canonical drive root (`C:\`). */ -function dirnameSimple(absolutePath: string): string { +function dirnameSimple(absolutePath: FilePath): FilePath { const sepIdx = Math.max(absolutePath.lastIndexOf('/'), absolutePath.lastIndexOf('\\')) - if (sepIdx > 0) return absolutePath.slice(0, sepIdx) - if (sepIdx === 0) return '/' + if (sepIdx > 0) { + const dir = absolutePath.slice(0, sepIdx) + return FilePathSchema.parse(/^[A-Za-z]:$/.test(dir) ? absolutePath.slice(0, sepIdx + 1) : dir) + } + if (sepIdx === 0) return FilePathSchema.parse('/') return absolutePath } @@ -224,5 +233,5 @@ export function fileUrlToPath(fileUrl: FileUrlString | URL): string { */ export function toSafeFileUrl(absolutePath: FilePath, ext: string | null): FileUrlString { const effectivePath = isDangerExt(ext) ? dirnameSimple(absolutePath) : absolutePath - return toFileUrl(effectivePath as FilePath) + return toFileUrl(effectivePath) } diff --git a/v2-refactor-temp/docs/file-manager/migration-plan.md b/v2-refactor-temp/docs/file-manager/migration-plan.md index 46f9399bb3e..da52d095d05 100644 --- a/v2-refactor-temp/docs/file-manager/migration-plan.md +++ b/v2-refactor-temp/docs/file-manager/migration-plan.md @@ -1539,7 +1539,7 @@ async function migrateFileEntry(oldFile: DexieFileRow): Promise { size: oldFile.size, externalPath: oldFile.origin === "external" - ? canonicalizeExternalPath(oldFile.path) + ? canonicalizeAbsolutePath(oldFile.path) : null, deletedAt: null, // Dexie 没软删除字段；external 也不允许 trashed（fe_external_no_delete） createdAt: toMs(oldFile.created_at), // ISO → ms @@ -1550,7 +1550,7 @@ async function migrateFileEntry(oldFile: DexieFileRow): Promise { **External path 去重（强制）**：新 schema 的 `UNIQUE(externalPath)` 禁止同路径两条行。如果 Dexie 里存在多条指向同一 canonical path 的 external FileMetadata（由 case / NFD / 拼写差异合并后），FileMigrator 必须： -1. 按 `canonicalizeExternalPath(path)` 分组 +1. 按 `canonicalizeAbsolutePath(path)` 分组 2. 每组保留一条（建议取 `createdAt` 最早的）作为 surviving row 3. 把组内其他 id 收集到 id-remap 表，在迁移 `file_ref`（以及其他引用 FileMetadata.id 的业务表）时将旧 id 重路由到 surviving id 4. 被合并掉的 FileMetadata.id 不产生 `file_entry` 行 @@ -1651,7 +1651,7 @@ ctx.sharedData.set('fileMigrator.idRemap', /* ReadonlyMap */ ctx.sharedData.set('fileMigrator.knownIds', /* ReadonlySet */) ``` -- **`idRemap`** 服务 external 去重——`canonicalizeExternalPath` 合并掉的 loser id 在表里映射到 surviving id。internal 文件**不在表里**（一对一保留）。 +- **`idRemap`** 服务 external 去重——`canonicalizeAbsolutePath` 合并掉的 loser id 在表里映射到 surviving id。internal 文件**不在表里**（一对一保留）。 - **`knownIds`** 是所有成功写入 `file_entry` 的 id 全集，供业务 migrator 校验 "这个 fileId 是否真的迁过去了"。 业务 migrator 通过这两份产物访问 file 子系统，**不直接查 DB**（保持迁移期单向数据流；同时避免对 fileEntryService 启动顺序产生隐式依赖）。 @@ -1698,7 +1698,7 @@ async function migrateOneMessageBlock(block) { | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | | `dexieExport.tableExists('files')` 返回 false | `recordWarning` 后跳过；FileMigrator 不产生任何 file_entry。业务 migrator 后续遇到 file id 查 `knownIds` miss → warn-skip ref（链式安全） | | 单条 file 字段缺失（`origin_name` undefined 等） | `recordWarning` + skip 该行；不中止迁移 | -| `canonicalizeExternalPath` 抛出（含 null byte） | **严重错误**：抛 `MigrationFatalError` 中止——v1 数据已被注入恶意路径，需人工调查 | +| `canonicalizeAbsolutePath` 抛出（含 null byte） | **严重错误**：抛 `MigrationFatalError` 中止——v1 数据已被注入恶意路径，需人工调查 | | External 去重时 surviving 选择失败 | 抛 `MigrationFatalError` 中止 | | `file_entry` INSERT 违反 schema CHECK（origin/size/externalPath 三元约束）| 抛 `MigrationFatalError` 中止——mapping 逻辑有 bug | | §2.10.2 物理抽样失败率 > 50% | 抛 `MigrationFatalError` 中止（前条文已规约） | diff --git a/v2-refactor-temp/docs/file-manager/rfc-file-manager.md b/v2-refactor-temp/docs/file-manager/rfc-file-manager.md index 0800656d6ab..edaf9aaa5c9 100644 --- a/v2-refactor-temp/docs/file-manager/rfc-file-manager.md +++ b/v2-refactor-temp/docs/file-manager/rfc-file-manager.md @@ -381,7 +381,7 @@ async function ensureExternalEntry( // Phase 1b.1 同步廉价 canonicalize: path.resolve + NFC + trailing-sep strip. // 不含 fs.realpath（case-insensitive FS 去重由 Phase 2 视用户反馈补）。 // 是 upsert/查询的唯一 key 来源。 - const canonicalPath = canonicalizeExternalPath(params.externalPath); + const canonicalPath = canonicalizeAbsolutePath(params.externalPath); // External 恒非 trashed（fe_external_no_delete CHECK），所以不需要 includeTrashed。 const existing = await fileEntryService.findByExternalPath(canonicalPath); if (existing) return existing; // name/ext 来自 externalPath，不会漂移；size 不存 @@ -1025,7 +1025,7 @@ Phase 1a ──→ Phase 1b.1 ──→ Phase 1b.2 ──→ Phase 1b.3 ── **不在本期**： - FileManager / ops / internal / watcher / danglingCache / orphanSweep 任何运行时逻辑 -- `canonicalizeExternalPath` 实现（契约与签名 Phase 1a 锁定，实现在 1b.1） +- `canonicalizeAbsolutePath` 实现（契约与签名 Phase 1a 锁定，实现在 1b.1） - `versionCache` 运行时（只定义 interface） - 任何 Dexie → SQLite 数据搬运 - renderer 切换调用路径 @@ -1041,7 +1041,7 @@ Phase 1a ──→ Phase 1b.1 ──→ Phase 1b.2 ──→ Phase 1b.3 ── - `FileEntryService` / `FileRefService` CRUD 实现（纯 DB；read 路径完整，write 可保留 stub） - `ops/fs.ts` 的 `read` / `stat` / `exists` / `metadata` / `contentHash`（xxhash-128） - `ops/path.ts` 的 `resolvePhysicalPath`（已存在）+ `isUnderInternalStorage` guard -- `canonicalizeExternalPath` 真实现（`path.resolve` + NFC + trailing-sep strip）+ 8-10 条边界测试（NFC/NFD / trailing / `./a/../a` / Windows `\\` / 盘符大小写） +- `canonicalizeAbsolutePath` 真实现（`path.resolve` + NFC + trailing-sep strip）+ 8-10 条边界测试（NFC/NFD / trailing / `./a/../a` / Windows `\\` / 盘符大小写） - `FileManager.get*` / `read*` / `getMetadata` / `getUrl` / `findByExternalPath` / `ensureExternalEntry`（upsert-only，不写 FS） - `internal/content/read.ts` / `internal/content/hash.ts`（含 `*ByPath` 变体） - `dispatchHandle(handle, byEntryFn, byPathFn)` helper 的读路径分派骨架 @@ -1199,7 +1199,7 @@ Vercel AI SDK Files API 稳定后： | Painting 的 file_ref 暂缺 | 文件页无法追溯 painting 引用 | 文件条目本身已存在可访问；随 Painting 重构补建 | | Phase 1 内 deferred-to-Phase-2 stub 的 `throw NotImplemented` 影响上游 | 开发期阻塞 | Phase 1 不切换 renderer 调用路径；剩余 stub（`fs.compressImage` / `path.resolvePath` / `path.isNotEmptyDir` / `shell.open` / `shell.showInFolder` / `search.listDirectory`）在 Phase 2 各自消费方迁移时一并实现并 feature-flag 切换 | | External entry 物理文件外部丢失 | entry 变 dangling | DanglingCache + File IPC `getDanglingState` / `batchGetDanglingStates` 给 UI 展示；不自动清理 file_ref（用户手动处理） | -| `externalPath` 大小写不敏感 FS 导致同文件双 entry（macOS APFS / Windows NTFS） | 文件页用户看到两份同文件、file_ref 分裂 | Phase 1b.1 `canonicalizeExternalPath` 做同步廉价规范化（resolve + NFC + trailing-sep），**刻意不做** `fs.realpath` case 去重——支配性来源（dialog / drag-drop）本就给 OS-canonical 值；收到真实用户报告后再 additively 扩展 + one-off migration 合并重复行 | +| `externalPath` 大小写不敏感 FS 导致同文件双 entry（macOS APFS / Windows NTFS） | 文件页用户看到两份同文件、file_ref 分裂 | Phase 1b.1 `canonicalizeAbsolutePath` 做同步廉价规范化（resolve + NFC + trailing-sep），**刻意不做** `fs.realpath` case 去重——支配性来源（dialog / drag-drop）本就给 OS-canonical 值；收到真实用户报告后再 additively 扩展 + one-off migration 合并重复行 | --- @@ -1358,7 +1358,7 @@ relinkExternalEntry(id: FileEntryId, newPath: FilePath): Promise; - **不触碰 FS**——与 `rename` 明确区分（"追认已移动" vs "主动移动"） - 保留 `id` 与所有 `file_ref` -- 调用 `canonicalizeExternalPath` 归一化新路径 +- 调用 `canonicalizeAbsolutePath` 归一化新路径 - 同步更新 DanglingCache 反向索引（旧 path 移除、新 path 添加、状态重置） - `name` / `ext` 作为 `externalPath` 的投影自动跟更新 diff --git a/v2-refactor-temp/docs/file-manager/utils-file-migration.md b/v2-refactor-temp/docs/file-manager/utils-file-migration.md index bbac0e85b2c..b3d376e27c9 100644 --- a/v2-refactor-temp/docs/file-manager/utils-file-migration.md +++ b/v2-refactor-temp/docs/file-manager/utils-file-migration.md @@ -18,7 +18,7 @@ src/main/utils/file/ ├── legacyFile.ts # v1 helpers（本文档规划如何把它拆干净） ├── fs.ts # v2 原语：read / write / stat / copy / move / remove / removeDir / atomicWriteFile / statVersion / contentHash ├── metadata.ts # v2 原语：getFileType(path) / isTextFile(path) / mimeToExt(mime) -├── path.ts # v2 原语：resolvePath / isPathInside / canWrite / isNotEmptyDir / canonicalizeExternalPath / resolvePhysicalPath / getExtSuffix +├── path.ts # v2 原语：resolvePath / isPathInside / canWrite / isNotEmptyDir / canonicalizeAbsolutePath / resolvePhysicalPath / getExtSuffix ├── search.ts # v2 原语：listDirectory (ripgrep + fuzzy) └── shell.ts # v2 原语：open / showInFolder ``` @@ -45,11 +45,11 @@ Notes 域（未来 `src/main/services/notes/`）承接 Notes 树扫描、笔记 | 函数 | Callers | 目标位置 | 新签名（建议） | Phase | 备注 | |---|---|---|---|---|---| -| `resolveAndValidatePath(baseDir, relativePath)` | 2 | `path.ts` | 同名，签名不变 | **1b.1** | 防路径穿越；`path.ts` 实现 canonicalizeExternalPath 时一起落实 | +| `resolveAndValidatePath(baseDir, relativePath)` | 2 | `path.ts` | 同名，签名不变 | **1b.1** | 防路径穿越；`path.ts` 实现 canonicalizeAbsolutePath 时一起落实 | | `untildify(pathWithTilde)` | 2 | `path.ts` | 同名 | **1b.1** | `~` 展开；纯 string op，无 FS | | `isPathInside(childPath, parentPath)` | 9 | `path.ts` | 同名 | **1b.1** | 已作为 `path.ts` 原语列在架构 §7；高 callers 数，保持 API 稳定 | -**合并手法**：Phase 1b.1 实现 `path.ts` 的 `canonicalizeExternalPath` / `isPathInside` 时，把上述函数原样内联进 `path.ts`，`legacyFile.ts` 留下 `export { ... } from './path'` 过渡 re-export 一轮（避免 9 处 `isPathInside` caller 一次性全改）。Phase 2 统一删 re-export。 +**合并手法**：Phase 1b.1 实现 `path.ts` 的 `canonicalizeAbsolutePath` / `isPathInside` 时，把上述函数原样内联进 `path.ts`，`legacyFile.ts` 留下 `export { ... } from './path'` 过渡 re-export 一轮（避免 9 处 `isPathInside` caller 一次性全改）。Phase 2 统一删 re-export。 ### 2.2 权限 / 存在性探测 → `@main/utils/file/path`