diff --git a/CHANGELOG.md b/CHANGELOG.md index 85af9e0..4b65934 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,14 @@ before it can remove or rename shared staging, backup, marker, or journal state; the persistent empty lock file is not a crash marker. The prepared installation handle is opaque so callers cannot forge its lock identity. + +- Make `settings-store` an opaque synchronized handle. The pre-1.0 API no + longer exports its constructor, path/lock/data fields, or mutation setters, + which allowed callers to bypass atomic persistence and replace the store's + lock. Transactions refresh under shared in-process and operating-system + locks to prevent stale handles from losing updates; stored JSON trees and + snapshots are deeply immutable. Use `make-settings-store` and the existing + controlled operations; `settings-store?` remains public. - Add a Windows `rivet::system::TrayController` that owns tray window subclassing, localized native menu presentation, command dispatch, and automatic `TaskbarCreated` recovery. The reference Taskboard now exercises diff --git a/docs/system-services.md b/docs/system-services.md index b22b58b..43791c2 100644 --- a/docs/system-services.md +++ b/docs/system-services.md @@ -77,6 +77,6 @@ The Linux integration binary self-checks the adapter (`RivetIntegration --system ## Settings, logs, and crashes -`make-settings-store` reads a JSON object and writes updates through an atomic replacement under a semaphore. The application chooses the path, normally its platform Application Support/AppData directory (on Linux, a file under the XDG state or config directories). Secrets do not belong in settings; use secure storage. +`make-settings-store` returns an opaque handle that reads a JSON object and writes updates through an atomic replacement under a shared in-process semaphore and operating-system file lock. Operations refresh inside the transaction, so multiple handles and cooperating processes do not overwrite one another from stale caches. Snapshots and stored JSON trees are deeply immutable; callers cannot mutate cached state around synchronization. The persistent empty `.lock` sibling carries no transaction state. Use `settings-ref`, `settings-set!`, `settings-remove!`, and `settings-snapshot`. The application chooses the path, normally its platform Application Support/AppData directory (on Linux, a file under the XDG state or config directories). Secrets do not belong in settings; use secure storage. `rivet-log` emits structured records to `current-rivet-log-sink`. `call-with-crash-reporting` records an escaping Racket exception and invokes `current-rivet-crash-reporter` before re-raising it. Native fatal hooks are deliberately minimal: crash handlers must avoid allocations and network access, then let a helper submit the report on the next launch. diff --git a/rivet/scribblings/rivet.scrbl b/rivet/scribblings/rivet.scrbl index 9eae130..74a25f8 100644 --- a/rivet/scribblings/rivet.scrbl +++ b/rivet/scribblings/rivet.scrbl @@ -272,12 +272,11 @@ Native hosts expose equivalent platform APIs in their native system library.} @defparam[current-system-adapter adapter system-adapter?]{ The adapter used by all system-service convenience procedures.} -@defstruct*[settings-store - ([path path?] - [lock semaphore?] - [data hash?]) - #:mutable]{ -Represents one synchronized, JSON-backed preferences file.} +@defproc[(settings-store? [value any/c]) boolean?]{ +Recognizes an opaque, synchronized JSON-backed preferences store. Its path, +lock, and cached data are private so updates cannot bypass the atomic public +operations. Transactions refresh under an in-process and operating-system +lock, and returned JSON trees are deeply immutable.} @defparam[current-rivet-log-sink sink procedure?]{ Receives each structured log record. The default writes JSON to the current @@ -297,11 +296,11 @@ secondary-instance activation.} [body string?] [#:tag tag (or/c #f string?) #f]) any/c]{ Displays an operating-system notification.} -@defproc[(set-tray-menu! [items list?]) any/c]{Configures a Windows tray or macOS menu-bar menu.} +@defproc[(set-tray-menu! [items list?]) any/c]{Configures a platform tray or menu-bar menu.} @defproc[(set-autostart! [enabled? boolean?]) any/c]{Enables or disables login startup.} @defproc[(autostart-enabled?) boolean?]{Reports the current login-startup state.} @defproc[(secure-store-set! [service string?] [account string?] [secret bytes?]) any/c]{ -Stores a secret through Windows Credential Manager or macOS Keychain.} +Stores a secret through the platform credential service.} @defproc[(secure-store-ref [service string?] [account string?] [default any/c #f]) any/c]{ Reads a secret, returning @racket[default] when no value is present.} @defproc[(secure-store-remove! [service string?] [account string?]) any/c]{Removes a stored secret.} diff --git a/rivet/system/settings.rkt b/rivet/system/settings.rkt index 6a7acef..6b3987a 100644 --- a/rivet/system/settings.rkt +++ b/rivet/system/settings.rkt @@ -5,14 +5,40 @@ racket/path racket/port) -(provide (struct-out settings-store) +(provide settings-store? make-settings-store settings-ref settings-set! settings-remove! settings-snapshot) -(struct settings-store (path lock data) #:mutable #:transparent) +;; The path, lock, and cached data form one synchronization invariant. Keep +;; the representation private so callers cannot replace any one of them and +;; then continue through the public operations with a corrupted store. +(struct settings-store (path lock data) #:mutable) + +(define store-locks (make-hash)) +(define store-locks-guard (make-semaphore 1)) + +(define (shared-store-lock path) + (define key (path->string (normal-case-path path))) + (call-with-semaphore + store-locks-guard + (lambda () + (hash-ref store-locks key + (lambda () + (define lock (make-semaphore 1)) + (hash-set! store-locks key lock) + lock))))) + +(define (immutable-settings-value value) + (cond + [(hash? value) + (for/hasheq ([(key item) (in-hash value)]) + (values key (immutable-settings-value item)))] + [(list? value) (map immutable-settings-value value)] + [(string? value) (string->immutable-string value)] + [else value])) (define (load-settings path) (cond @@ -24,15 +50,39 @@ (raise-arguments-error 'make-settings-store "settings file must contain a JSON object" "path" path)) - value])) + (immutable-settings-value value)])) (define (make-settings-store path) - (settings-store (path->complete-path path) - (make-semaphore 1) - (load-settings path))) + (define complete-path (path->complete-path path)) + (settings-store complete-path + (shared-store-lock complete-path) + (load-settings complete-path))) + +(define (store-lock-path store) + (path-add-extension (settings-store-path store) #".lock")) (define (with-store-lock store thunk) - (call-with-semaphore (settings-store-lock store) thunk)) + (call-with-semaphore + (settings-store-lock store) + (lambda () + (define lock-path (store-lock-path store)) + (make-parent-directory* lock-path) + (call-with-file-lock/timeout + #f + 'exclusive + thunk + (lambda () + (raise-arguments-error + 'settings-store + "another process owns the settings transaction" + "lock" lock-path)) + #:lock-file lock-path + #:max-delay 2.0)))) + +(define (refresh! store) + (define data (load-settings (settings-store-path store))) + (set-settings-store-data! store data) + data) (define (persist! store data) (define path (settings-store-path store)) @@ -54,25 +104,26 @@ (with-store-lock store (lambda () - (hash-ref (settings-store-data store) (normalize-key 'settings-ref key) default)))) + (hash-ref (refresh! store) (normalize-key 'settings-ref key) default)))) (define (settings-set! store key value) (with-store-lock store (lambda () + (define safe-value (immutable-settings-value value)) (persist! store - (hash-set (settings-store-data store) + (hash-set (refresh! store) (normalize-key 'settings-set! key) - value)) - value))) + safe-value)) + safe-value))) (define (settings-remove! store key) (with-store-lock store (lambda () (persist! store - (hash-remove (settings-store-data store) + (hash-remove (refresh! store) (normalize-key 'settings-remove! key)))))) (define (settings-snapshot store) - (with-store-lock store (lambda () (settings-store-data store)))) + (with-store-lock store (lambda () (refresh! store)))) diff --git a/tests/public-api.rkt b/tests/public-api.rkt index 1cdb6f9..ff1d18e 100644 --- a/tests/public-api.rkt +++ b/tests/public-api.rkt @@ -48,11 +48,9 @@ current-system-adapter install-crash-hook! make-settings-store register-activation-handler! rivet-log secure-store-ref secure-store-remove! secure-store-set! set-autostart! - set-settings-store-data! set-settings-store-lock! - set-settings-store-path! set-tray-menu! settings-ref settings-remove! - settings-set! settings-snapshot settings-store settings-store-data - settings-store-lock settings-store-path settings-store? - show-system-notification! struct:settings-store struct:system-adapter + set-tray-menu! settings-ref settings-remove! settings-set! + settings-snapshot settings-store? show-system-notification! + struct:system-adapter system-adapter system-adapter-acquire-single-instance system-adapter-autostart-enabled system-adapter-capabilities system-adapter-install-crash-hook system-adapter-name diff --git a/tests/system-services.rkt b/tests/system-services.rkt index aeb2f77..2ac7fa6 100644 --- a/tests/system-services.rkt +++ b/tests/system-services.rkt @@ -14,11 +14,32 @@ (check-equal? (settings-ref store 'theme "system") "system") (settings-set! store 'theme "dark") (check-equal? (settings-ref store "theme") "dark") + (check-true (settings-store? store)) + (check-true (immutable? (settings-snapshot store))) (define reloaded (make-settings-store settings-path)) (check-equal? (settings-ref reloaded 'theme) "dark") + ;; Two handles refresh under their shared transaction lock instead of + ;; overwriting one another from stale construction-time caches. + (settings-set! store 'first 1) + (settings-set! reloaded 'second 2) + (check-equal? (settings-ref store 'second) 2) + (check-equal? (settings-ref reloaded 'first) 1) + ;; Caller-owned mutable values are copied into an immutable JSON tree. + (define mutable-text (string-copy "before")) + (define mutable-nested (make-hasheq `((text . ,mutable-text)))) + (settings-set! store 'nested mutable-nested) + (string-set! mutable-text 0 #\X) + (hash-set! mutable-nested 'extra #t) + (define nested (settings-ref store 'nested)) + (check-true (immutable? nested)) + (check-true (immutable? (hash-ref nested 'text))) + (check-equal? nested (hasheq 'text "before")) (settings-remove! reloaded 'theme) (check-false (settings-ref reloaded 'theme #f))) - (lambda () (when (file-exists? settings-path) (delete-file settings-path)))) + (lambda () + (when (file-exists? settings-path) (delete-file settings-path)) + (define lock-path (path-add-extension settings-path #".lock")) + (when (file-exists? lock-path) (delete-file lock-path)))) (define secrets (make-hash)) (define notifications '())