security: more pentest fixes - #556
Merged
Merged
Conversation
lkacenja
added a commit
that referenced
this pull request
Aug 3, 2026
* security: fix security issues (#539) * security: authenticate and tenant-scope feedback endpoints FeedbackItemsController was unauthenticated and trusted a body-supplied user_id, allowing unauthenticated writes and user impersonation (pentest finding f-9a83). Inherit AuthenticatedController, derive user_id from current_user, and reject inferences outside the caller's site. Adds request specs (forgery protection disabled within that spec only, so it can reach the auth path) plus a factory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: keep header dropdown open on click The dropdown controller's document-level clickOutside handler fired on the same click that opened the menu and immediately re-hid it under real browser clicks (synthetic .click() was unaffected, which is why it passed manually but failed in Capybara). Stop propagation in toggle so the opening click never reaches the outside-click handler. Fixes the consistently-failing admin_spec.rb:24 feature test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * security: scope audit-report downloads to the site's bucket and prefix workflow_audit_report passed user-controlled bucket_name and key straight to S3, allowing cross-tenant reads and arbitrary-bucket access (pentest finding f-832d5cf3). Constrain the bucket to default_s3_bucket and the key to the requesting site's own reports/<machine_name>/ prefix, returning an identical 404 on mismatch so no bucket/key oracle leaks. Extract Site#machine_name (dedup) and guard against two sites resolving to the same slug. Remove the vestigial duplicate :id route segment so authorization pins to the actual site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * security: bump llm to 0.31.1 to fix code-injection vuln (CVE-2026-31236) ci/requirements.txt pinned llm==0.26, in the vulnerable range (<= 0.27.1) for GHSA-g76p-4vg5-f4qh. Bump to 0.31.1 (first fixed release was 0.28). This was the only pinned vulnerable copy: document_inference depends on llm only transitively via llm-gemini/llm-anthropic (llm>=0.26, unpinned), so its Docker build already resolves to the latest safe llm; evaluation has no llm dependency. Also aligns CI with the llm version prod actually installs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * security: more pentest fixes (#556) * Add failing test for reporting endpoint. * Add reporting endpoint to site protected list. * Add failing test for header manipulation attack. * Add fix for header manipulation attack. --------- 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
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.
We have two more high security findings from a recent penetration test. We should fix them to prevent any exploits from occurring. They are loosely related to a header manipulation vulnerability and a lack of site enforcement on the reporting endpoint. This PR addresses both.