fix(notify): sanitize operational logs - #100
Conversation
Keep Syncthing identifiers, local endpoints, folder markers, config paths, raw dependency errors, and HTTP response bodies out of helper logs. Preserve troubleshooting through fixed categories, counts, status codes, and bounded regression coverage without changing trigger v1 behavior.
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe notify service now uses bounded error categories and status-only HTTP errors, removes sensitive fields from runtime logs, adds privacy lint checks, verifies sanitized logging in tests, and documents the retained and excluded diagnostic data. ChangesNotify privacy-safe diagnostics
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
notify/main.go (1)
80-89: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEcho the config-load error before exiting
loadConfigAwaitingSyncthingerrors are collapsed to a generic slog entry here, so the actual missing-env-var / permission hint never reaches stderr. This runs before the mode switch, so--doctorand--healthchecknever get a chance to surface richer diagnostics. Printingerrhere would keep the operator-facing message without exposing secrets.🤖 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 `@notify/main.go` around lines 80 - 89, Update the loadConfigAwaitingSyncthing error path to include the original err details in the existing slog.Error call before os.Exit(1), while retaining the current classification, component, error_kind, and action fields.
🤖 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 `@notify/main.go`:
- Around line 80-89: Update the loadConfigAwaitingSyncthing error path to
include the original err details in the existing slog.Error call before
os.Exit(1), while retaining the current classification, component, error_kind,
and action fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 930364e1-488f-4332-9858-f7ae85ff8c89
📒 Files selected for processing (12)
CHANGELOG.mdPRIVACY.mddocs/relay-spec.mdios/scripts/sync-proof-privacy-lint.shnotify/README.mdnotify/doctor.gonotify/errors.gonotify/main.gonotify/main_test.gonotify/relay.gonotify/relay_test.gonotify/syncthing.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ CodeRabbit configuration file
**:
VaultSync
Self-hosted Obsidian vault sync for iPhone and iPad.
Your notes sync peer-to-peer over Syncthing, straight into Obsidian's iOS sandbox — no note cloud, no account, no tracking.
🔭 Why VaultSync
- Peer-to-peer & private — syncs directly between your own devices over Syncthing. No note cloud, no account, no tracking.
- Lands in Obsidian — files sync into Obsidian's iOS sandbox, where the app already looks for them.
- Pair by QR, resolve conflicts — connect your server in seconds; settle Markdown conflicts with side-by-side diffs.
- Server changes can wake your iPhone — optional Cloud Relay asks iOS to wake the app after the helper observes a server change. T...
Files:
docs/relay-spec.mdCHANGELOG.mdPRIVACY.mdios/scripts/sync-proof-privacy-lint.shnotify/README.mdnotify/relay_test.gonotify/doctor.gonotify/syncthing.gonotify/errors.gonotify/relay.gonotify/main_test.gonotify/main.go
docs/**
⚙️ CodeRabbit configuration file
docs/**: # ArchitectureVaultSync embeds Syncthing's Go reference implementation as an iOS library via gomobile — no reimplementation of the protocol in Swift, and guaranteed wire compatibility.
┌─────────────────────────────────┐ │ SwiftUI Frontend │ iOS-native UI, Swift 6 ├─────────────────────────────────┤ │ Swift ↔ Go Bridge │ thin API via gomobile │ │ → exported as .xcframework ├─────────────────────────────────┤ │ syncthing/lib (Go) │ protocol, discovery, sync └─────────────────────────────────┘ ↕ filesystem ┌─────────────────────────────────┐ │ Obsidian Vault (direct) │ Obsidian's iOS sandbox └─────────────────────────────────┘🔄 Sync strategy
- Foreground — Syncthing runs unrestricted: immediate, continuous sync.
- Background —
BGAppRefreshTask(requested ~15 min out; iOS decides the actual timing) +BGProcessingTask(overnight catch-up: multi-minute budget while charging with network) +BGContinuedProcessingTask(iOS 26+, longer runtime for user-initiated tasks). A ~30s grace window after backgrounding lets in-flight work finish.- Push (Cloud Relay) — optional. Near-realtime
server → iPhonewake-ups via APNs silent push. See relay-spec.md.VaultSync is intentionally asymmetric:
Direction Path Server → iPhone vaultsync-notifyspots outgoing changes → Cloud Relay silent push → VaultSync wakes and pulls.iPhone → Server iOS doesn't guarantee timely background execution for local edits. The reliable path is to open VaultSync and let embedded Syncthing run in the foreground — a Shortcuts automation can do that automatically whenever you leave Obsidian. Cloud Relay is a
server → iPhoneacceleration path, not a guarantee of symmetric real-time background sync.Relay and sync proof hierarchy
VaultSync models proof as independent fi...
Files:
docs/relay-spec.md
**/*
⚙️ CodeRabbit configuration file
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.
Files:
docs/relay-spec.mdCHANGELOG.mdPRIVACY.mdios/scripts/sync-proof-privacy-lint.shnotify/README.mdnotify/relay_test.gonotify/doctor.gonotify/syncthing.gonotify/errors.gonotify/relay.gonotify/main_test.gonotify/main.go
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.
Files:
docs/relay-spec.mdCHANGELOG.mdPRIVACY.mdnotify/README.md
notify/**/*.go
📄 CodeRabbit inference engine (README.md)
The Cloud Relay sidecar (
vaultsync-notify) should never access or log note content, file or folder names, or vault structure — only Device ID and push token
Files:
notify/relay_test.gonotify/doctor.gonotify/syncthing.gonotify/errors.gonotify/relay.gonotify/main_test.gonotify/main.go
⚙️ CodeRabbit configuration file
notify/**/*.go: Review goroutine lifecycle, context cancellation, HTTP timeouts, signal handling, debounce behavior,
Syncthing REST API polling, relay API calls, error classification, and API-key handling.
Flag leaked request bodies, note metadata, Syncthing API keys, relay keys, or APNs-related secrets.
Files:
notify/relay_test.gonotify/doctor.gonotify/syncthing.gonotify/errors.gonotify/relay.gonotify/main_test.gonotify/main.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-12T13:57:19.298Z
Learning: Trigger v1 must not be considered authenticated helper identity, APNs delivery, background execution, synchronization progress, upload, download, or roundtrip completion.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-12T13:57:19.298Z
Learning: The shipped App Store build cannot use a self-hosted relay; configurable `RELAY_URL` is intended for development or testing, while self-hosting requires a separately built app with its own bundle ID, Apple account, APNs key, and relay.
🔇 Additional comments (14)
CHANGELOG.md (1)
21-21: LGTM!PRIVACY.md (1)
67-74: LGTM!docs/relay-spec.md (1)
258-258: LGTM!notify/README.md (1)
178-182: LGTM!ios/scripts/sync-proof-privacy-lint.sh (3)
7-12: LGTM!
48-53: LGTM!
41-46: 🔒 Security & PrivacyNo issue:
notify/errors.gohas noslogcalls.> Likely an incorrect or invalid review comment.notify/errors.go (1)
3-20: LGTM!Also applies to: 22-56
notify/relay.go (1)
9-9: LGTM!Also applies to: 59-64, 123-128, 181-186, 207-225, 240-244
notify/syncthing.go (1)
7-7: LGTM!Also applies to: 65-73, 159-164, 261-268, 289-296
notify/relay_test.go (1)
7-33: LGTM!notify/doctor.go (1)
207-220: LGTM!Also applies to: 252-264, 279-286, 340-351, 373-380, 409-413
notify/main.go (1)
10-15: LGTM!Also applies to: 94-113, 223-227, 369-376, 381-396, 471-481, 534-540, 610-617, 651-657, 684-690, 694-700, 819-844, 847-852
notify/main_test.go (1)
7-7: LGTM!Also applies to: 828-832, 891-910
Preserve operator guidance for early configuration failures with fixed categories, whitelisted field names, and bounded remediation actions. Keep raw errors, user-provided values, and config paths out of operational logs while leaving configuration parsing and control flow unchanged.
|
Addressed the underlying early-startup diagnostics concern in e5d746f. Configuration failures now emit a fixed |
1 participant
This removes Syncthing Device IDs, local and Relay endpoint URLs, folder identifiers and event markers, config paths, raw dependency errors, and HTTP response bodies from
vaultsync-notifyoperational logs. The helper now emits fixed error categories, bounded counts, status codes, and durations while retaining the original in-memory errors for control flow and privacy-safe doctor remediation.HTTP dependency errors no longer retain URLs or response bodies, and the Privacy Policy, helper documentation, Relay specification, changelog, and sync-proof privacy lint describe and enforce the boundary. Regression coverage reproduces the previously leaking inactive-subscription startup path with Device-ID, URL, and body sentinels.
Trigger v1 requests, event selection, debounce behavior, configuration, persisted state, and Cloud Relay payloads are unchanged. The branch passed the complete Notify Go suite,
go vet,gofmt, ShellCheck 0.10.0, the sync-proof privacy lint,govulncheck, andgit diff --check.Summary
vaultsync-notifyoperational logging by replacing sensitive details and raw errors with fixed categories, status codes, bounded counts, scopes, and durations.Verification
Notify Go tests,
go vet,gofmt, ShellCheck, privacy lint,govulncheck, andgit diff --checkpassed.