fix: keep VPN usage as audit metadata - #861
Conversation
Continue address screening for VPN users and record unavailable VPN measurements as unknown. Preserve country restrictions and fail-closed address screening.
|
🚅 Deployed to the euler-lite-pr-861 environment in euler-lite(dev,PR previews)
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (7)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughVPN detection now returns ChangesVPN-aware address screening
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant WalletScreen
participant VPNDetection
participant AddressScreening
WalletScreen->>VPNDetection: Detect VPN usage
VPNDetection-->>WalletScreen: Return true, false, or null
WalletScreen->>AddressScreening: Send address and vpnIsUsed
AddressScreening-->>WalletScreen: Return screening verdict
WalletScreen->>WalletScreen: Allow or block from screening verdict
Suggested reviewers: Merge Risk: ⚪ Minimal · up to VPN results remain audit metadata while screening continues to control wallet access. The tested change is mergeable with no actionable current-head risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
composables/useAddressScreen.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. services/screening.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). services/vpn.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency).
Comment |
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review — approved
Reviewed exact head 40834da9f0fbc8904962466df1c69d94ce587f5a against development. No blocking findings.
Scope and walkthrough
- Reviewed all seven changed files: the address-screening composable, both client services, their three test files, and architecture documentation.
- VPN detection now distinguishes measured true/false from unavailable evidence, and no longer independently disconnects eligible addresses. All three values continue into address screening.
- Checked the adjacent server proxy, trusted-edge evidence precedence, country restriction middleware, and stale-generation guards. Address-screening failures remain fail-closed; client false/null cannot erase positive trusted-edge evidence.
- Cross-repo check: current Data v3 route accepts nullable VPN input, its screening service records it on audit paths rather than using it as the eligibility verdict, and its operations documentation describes the same contract. No consumer package/schema update is needed for this change.
- Whole-diff security pass: no dependency, credential, deployment configuration, or network-destination changes.
Validation
npx vitest run tests/composables/useAddressScreen.test.ts tests/services/vpn.test.ts tests/services/screening.test.ts tests/server/screen-address.test.ts tests/server/edge.test.ts tests/server/geo-gate.test.ts— 6 files, 86 tests passed.npm run test:run— 221 files, 2,164 tests passed.npm run typecheck— passed.npm run lint— 0 errors, 6 warnings in unchanged files.npm run build— passed.
The concrete screening fixture is the existing mocked address 0x0000000000000000000000000000000000000001, exercised with clean/restricted verdicts and true/false/null VPN evidence. This is source and automated-test validation, not a live provider call or wallet-browser smoke. Technical approval does not establish production deployment or replace any required policy-owner sign-off.
Summary
VPN usage is audit metadata. A positive VPN flag or an unavailable VPN measurement must not disconnect an otherwise eligible wallet; wallet access depends on the address-screening verdict.
Changes
null; preserve measuredtrueandfalsevalues.Test plan
npm run lint(0 errors; 6 existing warnings)npm run typechecknpm run buildnpm run test:run(221 files; 2,164 tests passed)No environment changes are required. Live wallet browser validation has not been performed for this change.
Summary by CodeRabbit
Behavior Changes
Documentation