fix: resolve CodeQL alerts (path injection + workflow token scope)#25
Merged
Merged
Conversation
Resolves the open CodeQL alerts on the repo: - js/path-injection (error): /deleteImage passed the client-supplied `imagePath` query param straight into `fs.rename`, so a value like "../../etc/passwd" could rename files outside the data directory. Resolve the path and verify it stays within data/ before any fs use, passing the confined absolute path to `fs.rename`. - actions/missing-workflow-permissions (warning): add a top-level `permissions: contents: read` block to the Test workflow so the GITHUB_TOKEN is narrowed from its broad default. Adds a test covering the traversal-rejection path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Apply the same data-directory confinement to the /search handler, whose client-supplied `imagePath` was read by the embedder. Extract the resolveWithinDataDir guard into utils/util.ts so deleteImage and query share one implementation, and cover the search traversal-rejection path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeQL still flagged js/path-injection because it only recognizes the data-directory check as a sanitizer when the guard is in the same function as the sink; the shared resolveWithinDataDir helper put it behind a function return, so the barrier didn't transfer. Inline the resolve-and-prefix-check guard directly in deleteImage and queryImages, keeping only DATA_DIR shared in utils/util.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The startsWith(DATA_DIR + path.sep) guard wasn't accepted by CodeQL's path-injection sanitizer recognition. Switch to the documented idiom — reject when path.relative(DATA_DIR, resolved) starts with ".." — which CodeQL treats as a path-traversal barrier. Semantics are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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
Fixes the open GitHub Code Scanning (CodeQL) alerts on the repo — one real vulnerability and one hardening warning.
1.
js/path-injection(error) —server/deleteImage.tsThe
DELETE /deleteImagehandler passed the client-suppliedimagePathquery param straight intofs.rename(imagePath, …). A caller could sendimagePath=../../somethingand rename files anywhere the server process can write, outside the intendeddata/directory.Fix: resolve the path and verify it stays inside
data/before any filesystem access, then rename using the confined absolute path. The Pinecone lookup still uses the original stored relative path (that's the value in the vector metadata), so search/delete behavior is unchanged for legitimate paths.2.
actions/missing-workflow-permissions(warning) —.github/workflows/test.ymlThe workflow didn't declare a
permissions:block, soGITHUB_TOKENgot the broad default scope. Addedpermissions: contents: readat the top level; both jobs only check out and test.Testing
../../etc/passwd) is rejected before touching disk or Pinecone.deleteImagetest to expect the resolved absolute path.npm run lint,npm run typecheck, and fullnpm run test:server(43 tests incl. live e2e delete) all pass.🤖 Generated with Claude Code