feat(storage): implement secure filesystem vault manager with atomic writes#22
Open
Chethan-Regala wants to merge 8 commits intoAOSSIE-Org:mainfrom
Open
Conversation
added 8 commits
March 12, 2026 23:18
WalkthroughA new storage module is introduced for managing filesystem-based Markdown vaults. It provides secure path resolution and validation, atomic write operations, full CRUD semantics for notes, and directory management with containment checks. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant VaultManager
participant PathValidator
participant SafeWrite
participant FileSystem as File System
Client->>VaultManager: updateNote(path, content)
VaultManager->>PathValidator: resolveVaultPath(root, path)
PathValidator-->>VaultManager: resolvedPath
VaultManager->>PathValidator: validateVaultPath(root, resolved)
alt Path valid and contained
PathValidator-->>VaultManager: ✓ validated
VaultManager->>SafeWrite: writeFileAtomic(resolved, content)
SafeWrite->>FileSystem: write to .tmp file
SafeWrite->>FileSystem: fsync (best-effort)
SafeWrite->>FileSystem: atomic rename .tmp → target
FileSystem-->>SafeWrite: ✓ success
SafeWrite-->>VaultManager: ✓ written
VaultManager-->>Client: ✓ note updated
else Path outside vault
PathValidator-->>VaultManager: ✗ error (outside root)
VaultManager-->>Client: ✗ error thrown
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/storage/package.json`:
- Around line 5-7: The package.json "main" currently points to index.js but your
build emits to dist/, so update package.json to point "main" to the emitted
entrypoint (e.g., "dist/index.js") and add a "types" field (e.g.,
"dist/index.d.ts") for TS consumers; also ensure your tsconfig.json has
"outDir": "dist" and "declaration": true so declaration files are emitted for
the "types" entry (verify the produced filenames match the values you put in
"main" and "types").
In `@apps/storage/src/demo/VaultDemo.ts`:
- Around line 65-69: The demo currently constructs VaultManager with
options.rootPath pointing to "demo-vault" without ensuring that directory exists
and only removes notes/ on cleanup; modify the flow to explicitly create the
demo root before new VaultManager(options) (use fs.mkdir or equivalent to ensure
options.rootPath exists), and move the teardown into a try/finally where the
finally calls fs.rm(options.rootPath, { recursive: true, force: true }) to
remove the entire demo root; update both the setup/teardown around VaultManager
instantiation (references: VaultOptions, options.rootPath, VaultManager) and the
other demo cleanup block (the notes/ removal at the later section) to rely on
the single root removal so the demo is rerunnable and leaves no residue.
In `@apps/storage/src/SafeWrite.ts`:
- Around line 80-95: After the rename(tempPath, filePath) call in SafeWrite.ts,
perform a best-effort sync of the parent directory (use path.dirname(filePath)),
by opening the directory with open(path.dirname(filePath), "r"), calling
dirHandle.sync(), and then closing dirHandle; wrap this in a try/catch that
ignores errors so platforms that don't support directory fsync don't fail the
write; keep the existing EPERM handling for Windows and ensure this
directory-sync step runs after the rename to provide the claimed crash-safety
for rename directory entries.
In `@apps/storage/src/types.ts`:
- Around line 232-244: The deleteFolder(path: string) contract must explicitly
forbid deleting the vault root: update the deleteFolder docs and implementation
to validate the resolved/normalized target and throw if it equals the vault root
or is a root-like input (e.g., "", ".", "/", or the resolved root path), and
also reject upward-traversal inputs (e.g., paths that resolve outside the
vault). In other words, inside deleteFolder ensure you normalize/resolve the
path against the vault root, check that it is not equal to the vault root and
that it remains inside the vault boundary, and if it fails these checks throw a
descriptive error instead of proceeding with recursive deletion.
- Around line 208-218: listNotes() currently leaves ordering undefined which
violates the storage layer's deterministic behavior requirement; change
implementations and the interface contract for listNotes() to return a
deterministically sorted array (e.g., sort VaultNoteMeta[] by a stable key such
as normalized note path, falling back to mtime for ties) and update any
callers/implementations to rely on that ordering; ensure the documentation
comment for listNotes() (and the VaultNoteMeta contract) states the exact sort
rule (e.g., "sorted ascending by normalized path, then by modification time") so
behavior is stable across platforms.
- Around line 150-206: The public API methods createNote, readNote, updateNote,
deleteNote, and renameNote currently accept arbitrary paths; update the contract
to require vault-relative Markdown paths (must end with ".md") by clarifying the
JSDoc for each method and adding a runtime validation requirement:
implementations must validate the input path endsWith(".md") and reject/throw a
clear error if not (also validate both oldPath and newPath in renameNote).
Mention this rule in the comments for createNote, readNote, updateNote,
deleteNote, and renameNote so callers and implementers enforce Markdown-only
note paths.
In `@apps/storage/src/VaultManager.ts`:
- Around line 112-123: When fs.realpath(resolved) throws ENOENT, don't skip
containment checks; instead walk up from resolved to find the nearest existing
ancestor, call fs.realpath on that ancestor and use that resolved ancestor path
when calling validateVaultPath(realRoot, realAncestorRealPath). Update the
try/catch in VaultManager where fs.realpath(this.options.rootPath) and
fs.realpath(resolved) are awaited so that ENOENT triggers the ancestor
resolution logic, then validate against the real root via validateVaultPath;
this prevents createNote/createFolder and rename destination paths from escaping
the vault via symlinks.
- Around line 311-313: deleteFolder currently resolves user input via
resolveSafe then calls fs.rm recursively, which allows callers to delete the
entire vault when folderPath is ""/"." or an absolute root; update deleteFolder
to detect and refuse any attempt to remove the vault root by comparing the
resolved path against the vault root (e.g., this.vaultRoot or whatever base path
is used by resolveSafe) and throw a clear error instead of calling fs.rm; ensure
you also normalize/realpath both sides (use path.resolve or fs.realpath) so
symlinks/relative forms like "." are caught, then only call fs.rm when the
resolved path is strictly inside the vault and not equal to the root.
- Around line 146-157: The preflight stat() check in createNote (around the
resolved variable before writeFileAtomic) and the identical pattern in
renameNote create a race where a concurrent writer can win between stat() and
write/rename; replace the stat()-then-write flow with an atomic create that
fails if the file exists (use fs.open(resolved, 'wx') or equivalent) so creation
is atomic and respects the no-overwrite contract, and adjust renameNote to
perform the destination-exclusive create/replace via an atomic primitive instead
of preflight stat() before fs.rename(); ensure you close the file descriptor and
handle the error path by propagating the "already exists" error when open('wx')
fails.
In `@apps/storage/tsconfig.json`:
- Around line 14-17: The base tsconfig.json currently sets "rootDir": "./src"
which causes "outside rootDir" errors for top-level tooling/tests; remove
"rootDir" from the base tsconfig.json and keep only non-emit settings (e.g.,
"outDir" can also be moved), then create a build-specific tsconfig (e.g.,
tsconfig.build.json) that extends the base and sets "rootDir": "./src" and
"outDir": "./dist"; also update "include" in the base tsconfig to ["**/*.ts"]
(or ["src/**/*.ts","**/*.ts"] as needed) so the base config covers all package
.ts files while the build config controls emit-specific options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: db44915d-1494-4250-b985-06fc06dbc076
⛔ Files ignored due to path filters (1)
apps/storage/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
apps/storage/package.jsonapps/storage/src/FileSystemUtils.tsapps/storage/src/PathValidator.tsapps/storage/src/SafeWrite.tsapps/storage/src/VaultManager.tsapps/storage/src/demo/VaultDemo.tsapps/storage/src/index.tsapps/storage/src/types.tsapps/storage/tsconfig.json
Author
|
Thanks for the suggestion ! I've updated the implementation to address this and pushed the fix in the latest commit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces the filesystem storage subsystem for Smart Notes.
The goal of this module is to provide a secure, deterministic, and filesystem-native vault layer for managing Markdown notes locally. Instead of relying on a database as the source of truth, Smart Notes follows a local-first architecture where Markdown files on disk remain canonical.
The storage layer provides a safe abstraction over the filesystem so that higher-level components (watchers, indexers, retrievers, and AI features) can interact with notes through a consistent service API.
Storage Architecture
Below is the architecture of the vault storage layer introduced in this PR.
Core components
VaultManager
VaultServicecontract.PathValidator
SafeWrite
temp file → renamestrategy.types.ts
Demo Runner
Module Structure
src/
├─ VaultManager.ts
├─ PathValidator.ts
├─ SafeWrite.ts
├─ types.ts
├─ index.ts
└─ demo/
└─ VaultDemo.ts
temp file
→ optional fsync
→ atomic rename
Build and run:
The demo demonstrates:
How This Fits Into Smart Notes
This storage module forms the foundation of the Smart Notes architecture.
It enables future subsystems such as:
Architecture flow:
Design Goals
This storage system was designed with the following principles:
The implementation intentionally relies only on Node.js filesystem APIs to keep the storage layer lightweight and maintainable.
Feedback
Feedback on the storage architecture, API design, or security considerations would be greatly appreciated.
Thanks for taking a look!
Summary by CodeRabbit
New Features
Chores