fix(audit): harden local browser boundaries#303
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPins CI/local Go tooling, verifies generated systemviews are committed, centralizes history limit clamping with tests, makes favicon cache close-safe, hardens the filter downloader (size/path limits + tests), restricts WebKit bridge to trusted URIs, updates generated templ attribute resolution, and adds destroy-safe input detach APIs with tests. ChangesBuild system, CI workflow, and documentation alignment
Search history pagination and query limit clamping
Favicon cache close-safe shutdown
Filter downloader security hardening: size limits, filename validation, and path escape prevention
WebKit native bridge origin validation and trusted-page restriction
Systemviews templ-generated attribute resolution
Browser window teardown and input handler destroy-safety
Watcher debounce timing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 105-106: The workflow runs the make target make verify-generated
in both the lint and test jobs, causing redundant verification; either keep the
duplication with an explicit comment explaining the defense-in-depth rationale
or remove one invocation so verification only runs once (e.g., keep it in the
lint job). Update the .github/workflows/ci.yml to either remove the run: make
verify-generated step from the test job (or from the lint job) or add a clear
inline comment near the verify-generated steps stating that duplication is
intentional for safety/defense-in-depth, referencing the verify-generated target
and the lint and test jobs so future editors understand the reason.
- Around line 70-71: The workflow hardcodes TOOL_BIN_DIR=/usr/local/bin when
running the Makefile target install-golangci-lint, which can be unwritable;
remove the override so the Makefile default (TOOL_BIN_DIR?=$(shell go env
GOPATH)/bin) is used, or set TOOL_BIN_DIR to the GOPATH-based path instead;
update the GitHub Action step that calls make install-golangci-lint to omit the
TOOL_BIN_DIR argument (or compute and pass the GOPATH/bin value) so the
Makefile’s default behavior is preserved.
In `@Makefile`:
- Around line 158-161: The Makefile target install-golangci-lint uses a
versioned path for the install script which can mismatch the passed
$(GOLANGCI_LINT_VERSION); update the install command in the
install-golangci-lint target to fetch the stable install script (use "master" in
the URL) and keep passing $(GOLANGCI_LINT_VERSION) as the argument so the
installer always runs the same script regardless of the version variable; modify
the install line that references GOLANGCI_LINT_VERSION and TOOL_BIN_DIR
accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f52cde3-92cc-4859-a98a-710dd32d2765
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.github/workflows/ci.ymlMakefileREADME.mddocs/installation.mdinternal/application/usecase/search_history.gointernal/application/usecase/search_history_test.gointernal/infrastructure/favicon/cache.gointernal/infrastructure/favicon/cache_test.gointernal/infrastructure/filtering/downloader.gointernal/infrastructure/filtering/downloader_test.gointernal/infrastructure/webkit/messaging.gointernal/infrastructure/webkit/messaging_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 52-56: The workflow currently uses the mutable tag
actions/setup-go@v6 in the "Set up pinned Go toolchain" step (and the equivalent
setup step in the test job) which should be pinned to an immutable commit SHA;
replace actions/setup-go@v6 with actions/setup-go@<commit-sha> (the full 40-char
SHA for the v6 tag) in both occurrences, using the gh api command shown in the
review to fetch the correct tag SHA (or copy the SHA from the official
actions/setup-go repo) so the workflow references an immutable commit instead of
a mutable tag.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4b00786-24d5-4f41-adab-59874277732f
📒 Files selected for processing (2)
.github/workflows/ci.ymlinternal/infrastructure/externaltheme/noctalia/watcher_test.go
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
1-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit least-privilege
permissionsfor the workflow token.This workflow still uses default token permissions; define explicit read-only permissions to reduce CI supply-chain blast radius.
🔒 Suggested patch
name: CI +permissions: + contents: read + on: push:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 3, Add an explicit least-privilege permissions block to the "CI" workflow by inserting a top-level permissions key (for example permissions: contents: read) immediately under the workflow header so the workflow token is limited to read-only access to the repository contents; update the CI workflow (name: CI) to include this permissions block rather than relying on default token permissions.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1-3: Add an explicit least-privilege permissions block to the "CI"
workflow by inserting a top-level permissions key (for example permissions:
contents: read) immediately under the workflow header so the workflow token is
limited to read-only access to the repository contents; update the CI workflow
(name: CI) to include this permissions block rather than relying on default
token permissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 229c29c2-eb2d-4200-837c-be833405d89e
📒 Files selected for processing (12)
.github/workflows/ci.ymlMakefileinternal/application/usecase/search_history.gointernal/ui/app.gointernal/ui/browser_window.gointernal/ui/browser_window_test.gointernal/ui/input/gesture.gointernal/ui/input/gesture_test.gointernal/ui/input/global_shortcuts.gointernal/ui/input/global_shortcuts_test.gointernal/ui/input/keyboard.gointernal/ui/input/keyboard_test.go
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
os.Root.dumb://pages.package-lock.json.Closes #286.
Closes #287.
Closes #288.
Closes #289.
Closes #291.
Closes #292.
Closes #293.
Closes #294.
Closes #298.
Closes #299.
Test Plan
go test ./...make verify-generatedgit diff --checkReview
Summary by CodeRabbit
Bug Fixes
Refactor
Chores
Tests