feat(notify): add dormant M5 upload attestation - #112
Conversation
Implement the Decision 024 upload-only protocol and helper attestor behind the existing dormant diagnostics boundary. Validate exact signed request/query chains through M4 confinement, atomically persist immutable attestations, enforce tuple/rate/restart limits, and add Go/Swift golden, model, privacy, crash, race, and restart coverage. Keep helper and app product entry points unchanged; no capability, listener, response, download, roundtrip, or Relay integration is enabled.
Add a build-tagged two-instance local Syncthing test with fresh random request material for request propagation, confined helper observation, durable attestation, pinned mock acceptance, and rejection of synchronized copies as evidence. Run it in CI with immutable action refs, read-only mounts, no network, dropped capabilities, no persisted checkout credentials, and a digest-pinned Go image. No test artifact is uploaded or published.
Document the upload-only test claim, compatibility matrix, privacy and retention behavior, rollback, unsupported runtime surfaces, and the remaining helper-first gates. VaultSync 2.0 remains NO-GO; download, roundtrip, runtime, packaging, publication, rollout, and release work remain unapproved.
📝 WalkthroughWalkthroughAdds a dormant Decision 024 upload-attestation foundation with strict CBOR and cryptographic validation, atomic helper persistence, Swift and Go tests, isolated Syncthing E2E coverage, documentation, privacy-boundary checks, and CI integration. ChangesM5 upload-attestation foundation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 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 review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
notify/diagnostics_upload_syncthing_e2e_test.go (1)
216-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
t.Context()overcontext.Background()in test helpers.Since Go 1.24,
testing.TB.Context()returns a context auto-canceled right beforet.Cleanupruns, which is a slightly more idiomatic fit thancontext.Background()for the various short-lived exec/HTTP calls throughout this file (e.g. Lines 216, 248, 491). Purely stylistic here since each call already has its own timeout — not blocking.🤖 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/diagnostics_upload_syncthing_e2e_test.go` around lines 216 - 218, Update the test helper call sites that create short-lived operation contexts, including the flow around exec.CommandContext, to derive them from t.Context() instead of context.Background(). Preserve each existing timeout and cleanup behavior, and apply the same change to the analogous call sites throughout the file..github/workflows/ci.yml (1)
106-109: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGate the Ubuntu test jobs on docs-only changes The docs-only skip currently applies only to
build;m5-syncthing-upload-e2eand the other non-macOS test jobs still run on documentation-only PRs. Add the sameneeds.changes/if: needs.changes.outputs.code == 'true'guard here if the goal is to save CI minutes.🤖 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 @.github/workflows/ci.yml around lines 106 - 109, Gate m5-syncthing-upload-e2e and the other non-macOS test jobs using the same needs.changes dependency and needs.changes.outputs.code == 'true' condition as build, so documentation-only changes skip these jobs while code changes continue to run them.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 106-109: Gate m5-syncthing-upload-e2e and the other non-macOS test
jobs using the same needs.changes dependency and needs.changes.outputs.code ==
'true' condition as build, so documentation-only changes skip these jobs while
code changes continue to run them.
In `@notify/diagnostics_upload_syncthing_e2e_test.go`:
- Around line 216-218: Update the test helper call sites that create short-lived
operation contexts, including the flow around exec.CommandContext, to derive
them from t.Context() instead of context.Background(). Preserve each existing
timeout and cleanup behavior, and apply the same change to the analogous call
sites throughout the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5aaa2867-2f9e-4f98-b170-e941df4bf1a0
📒 Files selected for processing (17)
.github/workflows/ci.ymlCHANGELOG.mdPRIVACY.mddocs/architecture.mddocs/m5-upload-attestation-readiness.mdios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swiftios/VaultSyncTests/Fixtures/diagnostics-upload-m5.jsonnotify/README.mdnotify/diagnostics_contract_model_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_namespace_filesystem_unsupported.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_protocol.gonotify/diagnostics_upload_protocol_test.gonotify/diagnostics_upload_syncthing_e2e_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**
⚙️ 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:
ios/VaultSyncTests/Fixtures/diagnostics-upload-m5.jsonnotify/diagnostics_namespace_filesystem_unsupported.goPRIVACY.mdCHANGELOG.mddocs/m5-upload-attestation-readiness.mdnotify/README.mdios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftdocs/architecture.mdnotify/diagnostics_upload_protocol.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_upload_syncthing_e2e_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_protocol_test.goios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
**/*
⚙️ 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:
ios/VaultSyncTests/Fixtures/diagnostics-upload-m5.jsonnotify/diagnostics_namespace_filesystem_unsupported.goPRIVACY.mdCHANGELOG.mddocs/m5-upload-attestation-readiness.mdnotify/README.mdios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftdocs/architecture.mdnotify/diagnostics_upload_protocol.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_upload_syncthing_e2e_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_protocol_test.goios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
notify/**/*.{sh,go}
📄 CodeRabbit inference engine (README.md)
The optional notify sidecar must support server-side wake-up requests and must not receive notes, file or folder names, or vault structure.
Files:
notify/diagnostics_namespace_filesystem_unsupported.gonotify/diagnostics_upload_protocol.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_upload_syncthing_e2e_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_protocol_test.go
**/*.{md,sh,go,swift}
📄 CodeRabbit inference engine (README.md)
Keep Relay-side request observation and wake-ups received on the iPhone as separate diagnostics evidence; one must not be treated as proof of the other.
Files:
notify/diagnostics_namespace_filesystem_unsupported.goPRIVACY.mdCHANGELOG.mddocs/m5-upload-attestation-readiness.mdnotify/README.mdios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftdocs/architecture.mdnotify/diagnostics_upload_protocol.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_upload_syncthing_e2e_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_protocol_test.goios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
notify/**/*.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/diagnostics_namespace_filesystem_unsupported.gonotify/diagnostics_upload_protocol.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_upload_syncthing_e2e_test.gonotify/diagnostics_namespace_filesystem_linux.gonotify/diagnostics_upload_attestor_linux_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_upload_protocol_test.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:
PRIVACY.mdCHANGELOG.mddocs/m5-upload-attestation-readiness.mdnotify/README.mddocs/architecture.md
docs/**/*.md
📄 CodeRabbit inference engine (docs/troubleshooting.md)
docs/**/*.md: After applying each troubleshooting fix, retry from the app to confirm whether the issue is resolved.
For a missing Device ID or “Sync Engine Not Running,” force-close and reopen VaultSync, keep it foregrounded for 20–30 seconds, confirm the Device ID, rescan the vault, and reboot if necessary.
vaultsync-notifymust read the Syncthing API key fromconfig.xml; permission or authentication failures should be diagnosed as wrong-file or read-permission problems before changing credentials.
Runvaultsync-notifyas the user owning Syncthing’s mode-0600config.xml, or configure the appropriate containerPUID/PGID.
SetSYNCTHING_CONFIGwhen the Syncthing configuration is not auto-detected, and remove or correctSYNCTHING_API_KEYoverrides that replace automatic detection.
For relay failures, verify internet access,RELAY_URL, server egress rules, relay health with curl, and the app’s relay diagnostics; confirm delivery through an updated “Last Trigger Received” timestamp.
A relay HTTP 429 from--doctorcounts as success because it proves the trigger endpoint is reachable; an inactive subscription is a warning fixed through app subscription or provisioning.
Peer-state warnings from--doctormust not fail the command: offline peers are normal, and--healthcheckmust skip peer-state diagnostics.
For missing pending shares, verify the desktop folder shares the exact iOS Device ID, ensure Syncthing is online and unpaused, refresh pending shares, and re-share if needed.
For APNs issues, retry APNs registration, verify a token appears, retry provisioning, and confirm delivery with “Last Trigger Received”; notification permission is not required for silent wake-ups.
When connecting Obsidian storage, chooseOn My iPhone → Obsidianor the vault root containing.obsidian, then confirm detected vaults or pending shares.
VaultSync must not move, recreate, or delete vault folders automatically; recovery from moved or deleted folders requires ma...
Files:
docs/m5-upload-attestation-readiness.mddocs/architecture.md
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/m5-upload-attestation-readiness.mddocs/architecture.md
**/*.swift
📄 CodeRabbit inference engine (Custom checks)
For Swift background execution changes, pass if work is bounded, cancellation-aware, handles expiration callbacks, and records errors without leaking private vault data. Fail only when background work can continue unbounded, miss cleanup, or violate iOS background execution constraints.
Files:
ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
ios/**/*.swift
📄 CodeRabbit inference engine (README.md)
ios/**/*.swift: Develop the iOS/iPadOS app using Swift 6 and SwiftUI, targeting iOS/iPadOS 18 or later.
Use VoiceOver and Dynamic Type throughout the iOS/iPadOS app.
Support localization in English, German, Spanish, and Simplified Chinese.
Files:
ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
⚙️ CodeRabbit configuration file
ios/**/*.swift: Focus on Swift 6 strict concurrency, Sendable/MainActor correctness, Task cancellation,
retain cycles, memory pressure, SwiftUI observation state, StoreKit/APNs flows, and iOS background execution limits.
Pay special attention to BGAppRefreshTask and BGContinuedProcessingTask behavior, expiration handling,
bounded work, and cleanup when the app is suspended or terminated.
Files:
ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
ios/**/*.{swift,plist}
📄 CodeRabbit inference engine (README.md)
ios/**/*.{swift,plist}: Use BGAppRefreshTask and BGContinuedProcessingTask when available for background processing, while allowing iOS to decide whether and when the app runs.
Implement APNs silent push handling for optional Cloud Relay wake-ups.
Files:
ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
.github/workflows/**/*.yml
⚙️ CodeRabbit configuration file
.github/workflows/**/*.yml: Review CI for correct Go test tags, Xcode/iOS simulator assumptions, secret scoping,
dependency integrity, permissions, Docker publishing safety, and unnecessary privilege escalation.
Files:
.github/workflows/ci.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Run Syncthing unrestricted in the foreground for immediate continuous synchronization.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Treat Cloud Relay as an optional server-to-iPhone acceleration path, not as a guarantee of symmetric real-time background synchronization.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Model synchronization proof as independent evidence fields; never derive stronger proof from weaker evidence or use one generic “sync succeeded” flag.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Do not treat relay reachability, trigger observation, push receipt, background start, engine reachability, scans, index updates, idle state, or 100% completion as local data progress; only a fresh successful incoming `ItemFinished` application qualifies.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Keep push receipt, background start, and organic background progress iPhone-wide and unattributed; only a manual check may scope fresh local evidence to one folder and its sole configured peer.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Manual synchronization checks must be passive: they may not create probe artifacts, rescan, write files, modify mappings or ignores, persist check identifiers/results, or run during onboarding, launch, silent push, or ordinary background sync.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Manual checks must enforce eligibility, freshness, generation, polling, staleness, cancellation, and conservative failure rules: only eligible single-peer folders qualify; evidence must be newer than the check start and baseline cursor; generation changes interrupt; results stale after 15 minutes; failures never produce false success.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Keep dormant pairing, namespace, and correlated-roundtrip foundations unreachable from production runtime; do not add installer, endpoint, listener, capability, folder-creation, or app-enablement paths without an authorized rollout decision.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:34.669Z
Learning: Do not implement upload, controlled-download, or full-roundtrip proof until the separately designed, capability-negotiated helper contract and safe app-owned diagnostics namespace are available.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Keep M5 upload-only and dormant: it must not authorize or provide helper runtime, App runtime, capability negotiation, packaging, publication, rollout, controlled download, or causal roundtrip; VaultSync 2.0 remains NO-GO.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Implement only Decision 024 deterministic CBOR messages 3–5: `operation_request`, `attestation_query`, and `upload_attestation`.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Require exact payload and authentication constraints, including 256-byte request payloads, SHA-256 domain/payload digests, Ed25519 signatures and key IDs, nonzero operation/nonces, exact bindings/epochs, and a 600-second TTL with 120-second clock skew.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Ensure helper reads requests only through the M4 confined root handle after validating authenticated namespace authorization.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Persist attestations atomically by writing and fsyncing an anonymous inode, linking it once to the final filename, fsyncing the directory, and returning the persisted bytes; partial final names and overwrites must be impossible.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Repeated exact queries and helper restarts may return only the exact authenticated persisted attestation; conflicting queries or artifacts must never upgrade evidence.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Enforce all stated operation, poll, start, and request rate limits, and count invalid requests against the applicable limits.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Swift parsing and upload acceptance must remain test-target-only and require the exact pinned mock channel and active query; pending state, HTTP acceptance, reachability, timestamps, and synchronized copies must not set upload.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Do not implement capability query/response, runtime flags, local endpoints, TLS, listeners, response authorization or artifacts, authenticated cleanup messages, download evidence, roundtrip, product UI, automatic discovery, namespace creation, pairing/trust adoption, or durable operation history in M5.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Preserve existing product behavior: current product app/helper combinations and existing setup and wire behavior must remain unchanged, and product state must require no migration.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Fail closed for rotated, revoked, lost, or mismatched keys, epochs, or bindings; require explicit re-pairing or re-authorization and never transfer trust automatically.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Cleanup may delete only a previously owned file whose descriptor-relative identity and digest still match; it must never target protected roots, metadata, credentials, user files, backups, history, or tombstones.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Do not add logging, telemetry, crash annotations, support exports, persistent credentials, production service clients, or production Syncthing configuration changes; E2E tests must use temporary isolated homes and folders.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: Rollback M5 by source revert only; remove test artifacts from temporary directories, and do not require product migration or wire rollback.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-13T18:23:41.835Z
Learning: After merge, compare Decisions 021–025 with the actual code, require separate owner scope approval for response/cleanup foundations, and retain independent PR and owner merge gates for later milestones.
📚 Learning: 2026-07-12T23:03:04.680Z
Learnt from: psimaker
Repo: psimaker/vaultsync PR: 107
File: ios/VaultSyncTests/DiagnosticsContractTests.swift:39-46
Timestamp: 2026-07-12T23:03:04.680Z
Learning: In iOS Swift tests that use CryptoKit’s `Curve25519.Signing.PrivateKey.signature(for:)` (Ed25519), don’t assert that a generated signature’s bytes exactly match deterministic “golden”/fixture signatures. CryptoKit signatures may be randomized (different but valid for the same key+message). Instead, verify correctness by calling `isValidSignature` (or equivalent) against (1) the golden bytes and (2) the freshly generated signature, and avoid byte-for-byte equality assertions between CryptoKit output and reference vectors.
Applied to files:
ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swiftios/VaultSyncTests/DiagnosticsUploadM5Tests.swift
🪛 ast-grep (0.44.1)
notify/diagnostics_upload_syncthing_e2e_test.go
[error] 227-230: An argument passed to exec.Command/exec.CommandContext is built by concatenating a string literal with dynamic input. If that input is attacker-controlled (and especially when the command is a shell such as sh -c/bash -c), this enables OS command injection. Pass untrusted data as separate, fixed arguments instead of interpolating it into a command string, avoid invoking a shell, and validate/escape the input where a shell is unavoidable.
Context: exec.Command(
binary, "serve", "--home", home, "--gui-address=http://"+guiAddress, "--gui-apikey="+instance.apiKey,
"--no-browser", "--no-restart", "--no-upgrade", "--no-port-probing", "--log-level=WARN", "--log-file=-",
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(command-injection-exec-concat-arg-go)
notify/diagnostics_namespace_filesystem_linux.go
[warning] 272-272: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: uint32(os.Geteuid())
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🔇 Additional comments (26)
notify/diagnostics_upload_syncthing_e2e_test.go (1)
1-119: LGTM!Also applies to: 121-177, 205-320, 341-507
notify/diagnostics_upload_protocol.go (3)
42-98: LGTM!
237-299: LGTM!
118-128: 🎯 Functional CorrectnessNo CBOR ordering gap here The Go decoder rejects non-canonical map encodings before
validateDiagnosticsUploadValueruns, so this sort does not introduce a Go/Swift parity gap.> Likely an incorrect or invalid review comment.docs/m5-upload-attestation-readiness.md (1)
1-95: LGTM!notify/diagnostics_upload_protocol_test.go (1)
63-345: LGTM!ios/VaultSyncTests/DiagnosticsUploadM5TestSupport.swift (3)
74-216: LGTM!
330-355: LGTM!
409-500: LGTM!ios/VaultSyncTests/DiagnosticsUploadM5Tests.swift (1)
7-304: LGTM!ios/VaultSyncTests/Fixtures/diagnostics-upload-m5.json (1)
1-33: LGTM!CHANGELOG.md (1)
11-11: LGTM!PRIVACY.md (1)
84-96: LGTM!docs/architecture.md (2)
94-94: LGTM!Also applies to: 146-171
134-145: 🎯 Functional CorrectnessNo change needed — these limits match
notify/diagnostics_upload_attestor.goandnotify/diagnostics_upload_protocol.go.> Likely an incorrect or invalid review comment.notify/README.md (1)
155-173: LGTM!notify/diagnostics_contract_model_test.go (2)
290-290: LGTM!Also applies to: 321-322, 365-366
406-458: LGTM!.github/workflows/ci.yml (2)
228-228: LGTM!
106-146: 🩺 Stability & AvailabilityNo issue: the containerized Go test stays self-contained. The pinned
golangimage matchesnotify/go.mod’sgo 1.26.5, andnotifyonly imports stdlib packages, so the--network none/ read-only setup does not block module or toolchain resolution here.> Likely an incorrect or invalid review comment.notify/diagnostics_namespace_filesystem_linux.go (3)
182-254: LGTM!
256-282: LGTM!
284-302: 🩺 Stability & AvailabilityThis path is part of the dormant diagnostics foundation, so the
linkat(AT_EMPTY_PATH)kernel caveat doesn’t apply to current product behavior.> Likely an incorrect or invalid review comment.notify/diagnostics_namespace_filesystem_unsupported.go (1)
21-23: LGTM!notify/diagnostics_upload_attestor.go (1)
168-461: LGTM!notify/diagnostics_upload_attestor_linux_test.go (1)
17-462: LGTM!
1 participant
Summary
Explicit boundaries
Verification
cd notify && go test ./... -count=1— pass on macOS (19.925s).go test ./... -count=1with no network, read-only root, dropped capabilities, andno-new-privileges— pass (15.704s).2.358s).cd notify && go vet ./...— pass.cd notify && go test -run '^$' -fuzz '^FuzzDiagnosticsUploadDecoder$' -fuzztime=10s— pass (58,415executions).govulncheck ./...innotify— no vulnerabilities found.8.313s); a synchronized attestation copy is rejected as evidence.go test -tags noassets ./bridge -count=1— pass.DiagnosticsUploadM5Testswith signing disabled — pass (32.269s).46.990s).8negative cases,1exact positive model case).actionlint v1.7.7— pass, excluding only its known unsupportedmacos-26runner-label diagnostic.759keys) — pass.Compatibility, privacy, and rollback
Release truth
VaultSync 2.0 remains NO-GO.