fix: guard unsupported AmneziaWG s3/s4 keys#33
Conversation
📝 WalkthroughWalkthrough
ChangesAmneziaWG s3/s4 suppression
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // The bundled amneziawg-go v1.0.4 UAPI does not accept s3/s4 yet. | ||
| // Keep these fields importable/storable, but do not emit them until | ||
| // the core supports the corresponding device keys. |
There was a problem hiding this comment.
The comment hard-codes the version number (v1.0.4) but there is no // TODO: marker or issue tracker link — so when the bundled amneziawg-go is bumped to a version that does accept s3/s4, the suppression will remain silently in place unless someone remembers to revisit this file. Adding a short TODO with a link to the upstream issue would make the re-enablement self-discoverable.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Closing as superseded by #34. This guard was correct only for the old bundled amneziawg-go core, where nonzero S3/S4 were unsupported. After #34, the app pins the updated sing-box/libcore with amneziawg-go v0.2.19, and we verified a real Android -> AWS AmneziaWG tunnel using nonzero S3/S4. Merging this PR now would reject the feature we just enabled. |
1 participant
Summary
Validation
Greptile Summary
This PR replaces a silent pass-through of unsupported
s3/s4AmneziaWG obfuscation parameters with an expliciterror()call that halts tunnel start-up and surfaces a descriptive message to the user. Import and in-database storage of these fields are unchanged.error()call throwsIllegalStateException, which propagates throughbuildConfig()and is caught bycatch (exc: Throwable)inBaseService.kt, causing the VPN service to stop with "Service failed: AmneziaWG S3/S4 are not supported by the bundled core" shown to the user.catch (e: Exception)block inConfigurationFragment.ktwhen a user attempts a sing-box JSON export of a profile with non-zeros3/s4, showing a snackbar with the error text — so that export path also fails gracefully.toStdLink,toUniversalLink) do not go throughbuildConfig, so those remain fully functional for profiles withs3/s4set.Confidence Score: 5/5
Safe to merge — the one-line guard is a strict improvement over the previous silent pass-through, and every call site already catches the thrown exception and presents a user-visible message.
The change is minimal and surgical: two lines are replaced by a guard that throws an explicit exception. All three call paths that reach this code (VPN service start, sing-box JSON export, URL test) already wrap the call in appropriate catch blocks, so the exception is handled gracefully in every case. Import and storage of s3/s4 values are unaffected.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User connects / exports sing-box config] --> B[buildConfig called] B --> C[buildSingBoxOutboundAmneziaWGBean] C --> D{bean.s3 != 0 or bean.s4 != 0?} D -- No --> E[Emit jc/jmin/jmax/s1/s2/h1-h4/i1-i5 normally] E --> F[Tunnel starts / config exported] D -- Yes --> G[error throws IllegalStateException] G --> H{Caller context?} H -- VPN service start BaseService.kt --> I[catch Throwable stopRunner with message] I --> J[User sees: Service failed: AmneziaWG S3/S4 not supported] H -- Sing-box JSON export ConfigurationFragment.kt --> K[catch Exception snackbar shown] K --> L[User sees snackbar error message] H -- URL test TestInstance.kt --> M[catch Exception returned to caller]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[User connects / exports sing-box config] --> B[buildConfig called] B --> C[buildSingBoxOutboundAmneziaWGBean] C --> D{bean.s3 != 0 or bean.s4 != 0?} D -- No --> E[Emit jc/jmin/jmax/s1/s2/h1-h4/i1-i5 normally] E --> F[Tunnel starts / config exported] D -- Yes --> G[error throws IllegalStateException] G --> H{Caller context?} H -- VPN service start BaseService.kt --> I[catch Throwable stopRunner with message] I --> J[User sees: Service failed: AmneziaWG S3/S4 not supported] H -- Sing-box JSON export ConfigurationFragment.kt --> K[catch Exception snackbar shown] K --> L[User sees snackbar error message] H -- URL test TestInstance.kt --> M[catch Exception returned to caller]Reviews (2): Last reviewed commit: "fix: reject unsupported nonzero AWG s3 s..." | Re-trigger Greptile