fix(api-server): address PR #189 review — security, encapsulation, error handling#192
Closed
fix(api-server): address PR #189 review — security, encapsulation, error handling#192
Conversation
GET /api/v1/sessions/:sessionId/history returns the full conversation history for a session from the context plugin's HistoryStore.
…al, 503 status - Add session existence check before querying history store - Add regex constraint to SessionIdParamSchema to prevent path traversal - Return 503 instead of 404 when history store is unavailable
…ror handling - Revert SessionIdParamSchema regex that was a breaking change for all session routes - Add path traversal protection in HistoryStore.filePath() using path.basename() - Remove decodeURIComponent bypass risk in history route - Add ServiceUnavailableError class for consistent 503 error handling - Refactor: route history access through ContextManager.getHistory() instead of exposing internal HistoryStore as standalone service - Add path traversal and ContextManager.getHistory() tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
Contributor
Author
|
Merged into develop via direct merge. |
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 critical issues identified in PR #189 (
feat/session-history-endpoint):SessionIdParamSchemaregex — the regex was applied to a shared schema used by ALL session routes, causing a breaking change. Path traversal protection is now handled at the data layer instead.HistoryStore.filePath()— usespath.basename()+ resolved path verification to prevent directory traversal attacks at the correct layer.decodeURIComponentbypass risk — regex validated the encoded form whiledecodeURIComponentcould produce different characters. Removed since nanoid IDs don't need URL decoding.ServiceUnavailableErrorclass — consistent 503 error handling through the global error handler instead of inlinereply.status(503).send().ContextManager.getHistory()— removed standalonehistory-storeservice registration. API server now accesses history through the existingcontextservice (ContextManager), keeping HistoryStore as an internal implementation detail.ContextManager.getHistory().Test plan
pnpm buildpassesGET /api/v1/sessions/:id/historyreturns history for active session🤖 Generated with Claude Code