-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V3 alpha feature/3659 support webview options #3766
V3 alpha feature/3659 support webview options #3766
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request includes updates to multiple Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Semgrep found 1 Risk: golang.org/x/net versions before 0.7.0 are vulnerable to Uncontrolled Resource Consumption caused by the HPACK decoder. A malformed HTTP/2 Stream could be sufficient to cause a denial of service from a small number of small requests. Manual Review Advice: A vulnerability from this advisory is reachable if you host a user-facing application utilizing net with the HPACK decoder Fix: Upgrade this library to at least version 0.7.0 at wails/mkdocs-website/generate/go.mod:33. Reference(s): GHSA-vvpx-j8f3-3w6h, CVE-2022-41723 Ignore this finding from ssc-912ef774-b507-401f-823e-0eae254d0f22.Semgrep found 1 Risk: Affected versions of github.com/go-git/go-git/v5 are vulnerable to Uncontrolled Resource Consumption. The vulnerability enables attackers to execute denial-of-service attacks by exploiting specially crafted responses from a malicious Git server, leading to resource exhaustion in go-git clients. Manual Review Advice: A vulnerability from this advisory is reachable if you are using go-git to communicate with untrusted servers and are not using the in-memory filesystem for storage Fix: Upgrade this library to at least version 5.11.0 at wails/mkdocs-website/generate/go.mod:15. Reference(s): GHSA-mw99-9chc-xw7r, CVE-2023-49568 Ignore this finding from ssc-678a0e08-5ae8-47ab-b49c-8d3eb7d55fee.Semgrep found 1 Risk: Affected versions of vite are vulnerable to Improper Handling Of Case Sensitivity / Exposure Of Sensitive Information To An Unauthorized Actor / Improper Access Control. The vulnerability arises when the Vite development server's option, Manual Review Advice: A vulnerability from this advisory is reachable if you host vite's development server on Windows, and you rely on Fix: Upgrade this library to at least version 4.5.2 at wails/v3/examples/dev/frontend/package-lock.json:615. Reference(s): GHSA-c24v-8rfc-w8vw, CVE-2023-34092, CVE-2024-23331 Ignore this finding from ssc-efa14576-9601-4ae6-939c-3da58aa25013.Semgrep found 1 Risk: Affected versions of golang.org/x/net are vulnerable to Uncontrolled Resource Consumption. The HTTP/2 vulnerability occurs when a malicious client rapidly creates and resets requests, causing excessive server resource consumption. Despite the MaxConcurrentStreams setting limiting the total requests, resetting an ongoing request enables the attacker to initiate a new request concurrently. Fix: Upgrade this library to at least version 0.17.0 at wails/mkdocs-website/generate/go.mod:33. Reference(s): GHSA-4374-p667-p6c8, CVE-2023-39325 Ignore this finding from ssc-59363580-1c3f-4351-96ad-c59c611f9c64.Semgrep found 7
Risk: Affected versions of golang.org/x/net, golang.org/x/net/http2, and net/http are vulnerable to Uncontrolled Resource Consumption. An attacker may cause an HTTP/2 endpoint to read arbitrary amounts of header data by sending an excessive number of CONTINUATION frames. Fix: Upgrade this library to at least version 0.23.0 at wails/v3/plugins/experimental/oauth/go.mod:52. Reference(s): GHSA-4v7x-pqxf-cx7m, CVE-2023-45288 Ignore this finding from ssc-46663897-ab0c-04dc-126b-07fe2ce42fb2. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
v3/pkg/application/webview_window_options.go (1)
297-301
: Enhance documentation for the new autofill optionsWhile the implementation looks good, the documentation could be more detailed to help developers understand:
- The security implications of enabling these features
- The default values of these options
- Any dependencies or requirements (e.g., WebView2 version)
Consider expanding the comments like this:
- // GeneralAutofillEnabled enables general autofill + // GeneralAutofillEnabled enables general form autofill in WebView2. + // When enabled, the browser will offer to save and autofill form data like addresses. + // Default: false GeneralAutofillEnabled bool - // PasswordAutosaveEnabled enables autosaving passwords + // PasswordAutosaveEnabled enables password autosave in WebView2. + // When enabled, the browser will offer to save passwords when users sign in to websites. + // Note: This feature requires a secure context (HTTPS) on the target website. + // Default: false PasswordAutosaveEnabled boolv3/pkg/application/webview_window_windows.go (2)
1503-1508
: Consider enhancing error handling for general autofill.The error handling could be improved to provide more context about what failed.
Consider this improvement:
if opts.GeneralAutofillEnabled { err = chromium.PutIsGeneralAutofillEnabled(true) if err != nil { - globalApplication.error(err.Error()) + globalApplication.error("Failed to enable general autofill: %v", err) } }
1510-1515
: Consider enhancing error handling for password autosave.Similar to the general autofill block, the error handling could be improved to provide more context.
Consider this improvement:
if opts.PasswordAutosaveEnabled { err = chromium.PutIsPasswordAutosaveEnabled(true) if err != nil { - globalApplication.error(err.Error()) + globalApplication.error("Failed to enable password autosave: %v", err) } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
mkdocs-website/generate/go.sum
is excluded by!**/*.sum
v3/examples/dev/frontend/package-lock.json
is excluded by!**/package-lock.json
v3/plugins/experimental/oauth/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (6)
mkdocs-website/generate/go.mod
(1 hunks)v3/examples/dev/frontend/package.json
(1 hunks)v3/go.mod
(1 hunks)v3/pkg/application/webview_window_options.go
(1 hunks)v3/pkg/application/webview_window_windows.go
(1 hunks)v3/plugins/experimental/oauth/go.mod
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- v3/go.mod
🔇 Additional comments (10)
mkdocs-website/generate/go.mod (5)
8-8
: Note the replacement of imdario/mergo with dario.cat/mergo.
The change from github.com/imdario/mergo
to dario.cat/mergo
is a known migration path. This is the official new import path for the package.
Also applies to: 32-33
12-12
: Verify UI-related dependency updates.
Several UI and formatting related packages have been updated:
github.com/cheggaaa/pb/v3
(progress bar)github.com/fatih/color
github.com/mattn/go-colorable
github.com/mattn/go-isatty
github.com/mattn/go-runewidth
These updates should improve terminal output handling and color support.
Also applies to: 16-16, 23-25
9-10
: Review security-sensitive dependency updates.
Several security-sensitive packages have been updated:
github.com/Microsoft/go-winio
v0.6.2github.aaakk.us.kg/ProtonMail/go-crypto
v1.0.0github.aaakk.us.kg/cloudflare/circl
v1.4.0 (new cryptographic library)
These changes are good for security but warrant careful testing.
#!/bin/bash
# Check for any known vulnerabilities in the updated versions
echo "Checking for known vulnerabilities..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/Microsoft/go-winio") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/ProtonMail/go-crypto") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
Also applies to: 13-14
19-19
: Verify compatibility with major go-git updates.
The update to go-git/v5
and related packages (gcfg
, go-billy/v5
) represents significant version changes:
github.com/go-git/go-git/v5 v5.3.0 -> v5.12.0
github.com/go-git/gcfg v1.5.0 -> v1.5.1-0.20230307220236-3a3c6141e376
github.com/go-git/go-billy/v5 v5.3.1 -> v5.5.0
#!/bin/bash
# Check for go-git usage patterns that might be affected
echo "Checking for go-git usage patterns..."
# Search for git operations that might be affected by the update
rg -t go "git\.(Clone|Pull|Push|Fetch)" -A 3
# Search for custom config usage that might be affected by gcfg update
rg -t go "gcfg\." -A 3
Also applies to: 17-18
37-40
: Verify compatibility with updated core Go packages.
The updates to core Go packages (crypto
, net
, sys
, term
) represent significant version jumps. While these updates are good for security, they might introduce breaking changes.
✅ Verification successful
Core Go package updates are safe to proceed
The verification shows that:
- Only
net/url
is directly imported, and it's used in a simple, stable way for URL construction - All core packages (
crypto
,sys
,term
) are marked as indirect dependencies - These packages are pulled in by other direct dependencies (like
go-git/v5
,logrus
, etc.) - The version updates are minor and don't introduce breaking changes to the APIs being used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any Go files using deprecated APIs from these packages
echo "Checking for potential breaking changes in Go core package usage..."
# Search for crypto package usage
rg -t go "crypto\." -A 3
# Search for net package usage
rg -t go "net\." -A 3
# Search for sys package usage
rg -t go "sys\." -A 3
# Search for term package usage
rg -t go "term\." -A 3
Length of output: 826
Script:
#!/bin/bash
# Let's check for direct imports of these packages first
echo "Checking for imports of core Go packages..."
# Search for imports of crypto packages
rg -t go '^import.*"crypto/' -B 1 -A 3
# Search for imports of net packages
rg -t go '^import.*"net/' -B 1 -A 3
# Search for imports of sys packages
rg -t go '^import.*"sys/' -B 1 -A 3
# Search for imports of term packages
rg -t go '^import.*"term/' -B 1 -A 3
# Also check go.mod to see if these are truly indirect dependencies
echo -e "\nChecking module dependencies..."
cat mkdocs-website/generate/go.mod
Length of output: 3105
v3/plugins/experimental/oauth/go.mod (3)
9-11
: Verify breaking changes in core dependencies
Significant version updates detected. Please ensure:
- No breaking changes in gorilla/sessions v1.4.0 and markbates/goth v1.80.0
- The wails alpha.7 update includes the required webview options for autofill
#!/bin/bash
# Check for any existing usage of deprecated/removed features
rg -A 3 "sessions\."
rg -A 3 "goth\."
# Check if other modules are aligned with wails v3.0.0-alpha.7
fd -e mod -x grep -l "github.com/wailsapp/wails/v3"
51-61
: Verify security package versions
Several security-related packages have been updated. Please ensure:
- The crypto package version (v0.27.0) is sufficient for oauth security requirements
- The oauth2 package version (v0.23.0) supports all required oauth flows
#!/bin/bash
# Check for any security-related TODOs or FIXMEs
rg -i "TODO.*security|FIXME.*security"
# Check for minimum version requirements in other security-dependent modules
fd -e mod -x grep -l "golang.org/x/crypto\|golang.org/x/oauth2"
3-5
: Verify Go 1.23 compatibility requirements
Go 1.23 is currently in beta/RC stage. While this is acceptable for experimental modules, please ensure:
- All developers and CI systems can support Go 1.23
- There's a clear upgrade path for stable release
v3/pkg/application/webview_window_options.go (1)
296-301
: LGTM! The implementation aligns with the PR objectives
The new fields are appropriately placed in the WindowsWindow
struct and follow Go naming conventions. This implementation successfully addresses the autofill functionality request from issue #3732.
A few positive notes:
- The boolean type is appropriate for these toggle options
- The field names are clear and self-descriptive
- The placement in the Windows-specific options struct is correct since this is WebView2-specific functionality
Let's verify if there are any similar autofill options in other WebView implementations:
✅ Verification successful
No equivalent autofill options found in other WebView implementations
The implementation is indeed Windows-specific as these autofill options are only present in the WebView2 (Windows) related files:
webview_window_options.go
: Definition of the optionswebview_window_windows.go
: Implementation of the options
This confirms that the placement in the Windows-specific options struct is correct, as these features are WebView2-specific and not available in other platform implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for autofill-related code in other platform-specific files
rg -i "autofill|password.*save" --type go
Length of output: 743
v3/pkg/application/webview_window_windows.go (1)
1503-1515
: Verify WebView2 version compatibility for autofill features.
These autofill features might require a minimum WebView2 version. We should verify compatibility before enabling them.
@@ -11,6 +11,6 @@ | |||
"devDependencies": { | |||
"@sveltejs/vite-plugin-svelte": "^2.0.0", | |||
"svelte": "^3.54.0", | |||
"vite": "^4.0.0" | |||
"vite": "^4.5.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Vite 4.5.2 is not the latest version in the 4.x range - update recommended
The version should be updated to 4.5.5 which is the latest in the 4.x series for better security and stability.
🔗 Analysis chain
LGTM! Good security practice updating vite.
The update from ^4.0.0 to ^4.5.2 is a good security practice as it includes important security fixes.
Let's verify if this is the latest secure version:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the latest vite version and any security advisories
# Expected: Confirm if 4.5.2 is the latest secure version in the 4.x range
# Check npm for latest versions
npm view vite versions --json | jq '.[] | select(startswith("4."))'
# Check for any security advisories
npm audit --json [email protected] | jq '.advisories'
Length of output: 1357
Description
Adds the
PasswordAutosaveEnabled
andGeneralAutofillEnabled
options for Windows.Fixes #3732
Summary by CodeRabbit
New Features
wails3 generate runtime
command.Dependency Updates
vite
to version^4.5.2
andgithub.aaakk.us.kg/wailsapp/go-webview2
to versionv1.0.17
.1.22
to1.23
in the OAuth module.Bug Fixes
AlwaysOnTop
feature on macOS and fixed compilation issues for aarch64 on Linux.