Security fixes combined (#243-#252) — staging test#263
Open
jessie-coco wants to merge 11 commits intodevelopfrom
Open
Security fixes combined (#243-#252) — staging test#263jessie-coco wants to merge 11 commits intodevelopfrom
jessie-coco wants to merge 11 commits intodevelopfrom
Conversation
Replace LIKE pattern matching with SQLite json_each() for proper JSON array searching. The previous approach interpolated the tag parameter into a LIKE pattern, allowing SQL wildcards (%, _) and double-quotes to alter query semantics and leak data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Restrict externally_connectable localhost to port 3462 only
- GET_AUTH_STATE: return { authenticated, user } instead of raw JWT token
- DASHBOARD_LOGIN: validate token with server before accepting
- Dashboard: remove extension token auto-sync (use own OAuth flow)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add single-quote escaping to escapeHtml() (') - Escape err.message in error handlers (lines 119, 137) - Escape item.type, item.status, item.priority, item.created_by in list and thread views - Escape item.id in data-id attribute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
V1 item handlers (handleGetItem, handleAddMessage, handleAssignItem,
handleResolveItem, handleVerifyItem, handleReopenItem, handleCloseItem,
handleRespondToItem) did not verify that the requested item belongs to
the authenticated user's app_id. This allowed any authenticated user to
read, modify, or close items belonging to other apps.
Add the same app_id scoping check used by V2 endpoints:
if (req.v2Auth?.app_id && item.app_id !== req.v2Auth.app_id)
→ 403 Forbidden
For handlers that previously skipped the item fetch (assign, resolve,
verify, reopen, close), an explicit getItem() call is added before the
scope check.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
V1 item handlers (handleGetItem, handleAddMessage, handleAssignItem,
handleResolveItem, handleVerifyItem, handleReopenItem, handleCloseItem,
handleRespondToItem) did not verify that the requested item belongs to
the authenticated user's app_id. This allowed any authenticated user to
read, modify, or close items belonging to other apps.
Add the same app_id scoping check used by V2 endpoints:
if (req.v2Auth?.app_id && item.app_id !== req.v2Auth.app_id)
→ 403 Forbidden
For handlers that previously skipped the item fetch (assign, resolve,
verify, reopen, close), an explicit getItem() call is added before the
scope check.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both GET /api/v2/distributions/:item_id and POST .../retry had a flawed access check that only ran when `item` was truthy. When an item was deleted (null), the check was skipped entirely, leaking dispatch logs and allowing retry triggers for any authenticated user. Add an explicit null check before the access control guard so deleted items return 404 immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oss (#250) Add an in-memory per-document mutex (promise chain) that serializes all read-modify-write operations on discussion JSON files. All four mutating endpoints (POST /discussions, POST /respond, POST /discussions/resolve, POST /submit-reply) are now wrapped in withDiscussionLock(doc, fn) so concurrent requests on the same document are queued instead of racing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add v2Auth middleware to GET /api/v2/adapters (was unauthenticated) - Encrypt endpoint configs at rest using the same encrypt/decrypt pattern already used for user_auths credentials - Decrypt on read in getEndpoint, getEndpoints, updateEndpoint, setEndpointDefault Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ing (#252) - Validate URL scheme (https?://) before window.open to prevent javascript: XSS - Mask credential values in auth settings table (show last 4 chars only) - Fix double-fetch auth using wrong property names (authToken/authUser → token/user) - Wrap new URL() in try/catch to prevent crash on malformed source_url Co-Authored-By: Claude Opus 4.6 <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
Combined branch merging all 9 security fix PRs for staging E2E testing before individual merges.
Included PRs
fix/243-sql-injection-tag-queryfix/244-v1-api-access-controlfix/245-sidepanel-xssfix/246-auth-token-exposurefix/248-dispatch-log-null-checkfix/249-dockerignore-securityfix/250-discussion-race-conditionfix/251-adapters-auth-encryptionfix/252-dashboard-xss-credentialsPurpose
This PR is for staging E2E testing only. It combines all security fixes into a single branch so they can be validated together before merging the individual PRs.
Conflict Resolution
One conflict in
dashboard/src/main.jswas resolved by keeping the security fix (removing extension auth token sync, per #246).