fix: fail closed on unknown privacy settings - #223
Conversation
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Changes설정 검증
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change is localized to strict privacy-settings parsing and fails closed for unknown or ambiguous values; no actionable merge-blocking risk remains beyond normal required checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
opencode-agent
Bot
added
priority: medium
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| require_exactly_one_path "$artifact_root/${expected_dirs[0]}/bundle/deb/*.deb" 'Debian bundle' | ||
| require_exactly_one_path "$artifact_root/${expected_dirs[0]}/bundle/appimage/*.AppImage" 'AppImage bundle' | ||
| require_exactly_one_path "$artifact_root/${expected_dirs[1]}/bundle/msi/*.msi" 'Windows MSI bundle' | ||
| require_exactly_one_path "$artifact_root/${expected_dirs[1]}/bundle/nsis/*.exe" 'Windows NSIS bundle' | ||
| require_exactly_one_path "$artifact_root/${expected_dirs[2]}/bundle/dmg/*.dmg" 'macOS DMG bundle' |
There was a problem hiding this comment.
📝 Info: Anchored path patterns tighten per-platform verification
require_exactly_one_path calls changed from unanchored globs to patterns anchored to specific expected_dirs (verify-release-artifacts.sh:58-62). Since find prints paths beginning with $artifact_root and the patterns are double-quoted, -path still matches. This scopes each bundle check to its platform directory — a tightening, not a regression.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Verify downloaded release artifact contract | ||
| shell: bash | ||
| run: bash .github/scripts/verify-release-artifacts.sh release-artifacts "${{ github.run_attempt }}" |
There was a problem hiding this comment.
📝 Info: Artifact counts stay consistent across the new verify step
The attest-release verify step runs before SBOM generation, where the script expects 17 files and 3 directories (6+6+5). The later inline checksum step expects 18, correctly counting the added disksage.spdx.json (release.yml:361). The windows-2022 directory matches the build matrix.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Purpose
Fail closed to DiskSage's offline privacy default when a persisted settings document contains an unknown/stale or structurally ambiguous
online_modevalue instead of silently preserving network authority.Exact current state
05651e03a63e2f81dc78c8aa7dfd1f678afce458.main:be418bac9bfbdc8aa41b1a698713833e3dbccfdc.src-tauri/src/settings.rs.31927075860, Release31927075902, Security Scan31927075863, and SAST Semgrep31927075911.Test-first lineage
The branch first added a regression proving that
{"online_mode":true,"unexpected_remote_setting":true}must fail closed toSettings::default()while production Serde still accepted unknown fields. The implementation then added only#[serde(deny_unknown_fields)]toSettings.The current exact head adds regressions for two additional ambiguous-authority classes without changing production behavior:
online_modekeys fail closed rather than accepting one duplicate value;null, numeric, and string representations ofonline_modefail closed rather than being coerced into network authority.Current production remains deliberately small:
serde_json::from_str(json).unwrap_or_default()and a bool-only settings schema. Missingonline_modeis valid and defaults offline; recognized explicitonline_mode: trueremains valid.Security / compatibility boundary
Current review evidence
CodeRabbit's most recent completed review of the implementation lineage reported no actionable comments for
src-tauri/src/settings.rs; a later incremental review request was rate-limited and therefore is not qualifying approval evidence. Combined status currently includes CodeRabbit success, but status/model evidence is not an independent formal approval.Remaining integration gates
Do not merge based on native green checks alone. Keep Draft until the unchanged exact head also satisfies every live central required workflow, the current one-approval/last-push rule, zero valid unresolved findings, fresh protected-main ancestry, and the repository-wide exact 100% owned-production region/statement-equivalent, branch, function, and line coverage contract owned by #156 without exclusions or threshold weakening. Pending, queued, skipped-required, neutral-required, failed, stale, predecessor, synthetic, diagnostic-only, status-only, model-only, author-only, rate-limited, no-source-scanner, or infrastructure-only evidence is not passing.
Summary by CodeRabbit