fix(connectivity): guard BLE preference writes by meganetaaan · Pull Request #528 · stack-chan/stack-chan

meganetaaan · 2026-07-08T00:03:44Z

概要

BLE preference server が任意の Preference 書き込みを受け付けないよう、書き込み window と whitelist を追加します。

変更内容

BLE preference 書き込みをデフォルト拒否
setup mode 中だけ明示的に write window を有効化
PREF_KEYS ベースで domain/key を whitelist 検証
純粋 validation helper と unit test を追加
unknown domain / unknown key / window 期限切れの拒否を検証

検証

cd firmware && npm run lint: pass
cd firmware && npm run test:unit: pass, 32 tests
cd firmware && npm run check:architecture: pass, 13 tests
git diff --check: pass

未実施

実機 BLE での動作確認
Moddable 実ビルド / test:moddable

リリース影響

patch。脆弱な書き込み経路を制限する修正のため、リリースノート記載が必要です。

Closes #505
関連 #399

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- BLE preference changes can now be enabled in a controlled write window.
- Added support for safely applying only approved preference updates.
Bug Fixes
- Preference writes are now blocked until write access is explicitly enabled.
- Invalid or unsupported preference changes are rejected instead of being applied.
- Write access can automatically close again after a set time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-08T00:03:50Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c8b95804-edaa-4c22-a191-3cb4467055c4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds a BLE preference write validation mechanism: a new preference-write-guard.ts module enforces a key whitelist and write-enabled checks, PreferenceServer gains enableWrites/disableWrites with an opt-in write-window timer, setup-mode wires it in, and tests plus tsconfig aliases and a UARTServer fake support the new behavior.

Changes

BLE Preference Write Guard

Layer / File(s)	Summary
Preference write validation module `firmware/host/modules/connectivity/preference-write-guard.ts`	Adds `PreferenceKeyList`, `PreferenceWriteRejection`, `PreferenceWriteDecision` types, `isAllowedPreferenceKey()`, and `validatePreferenceWrite()` to reject writes when disabled or the domain/key is not whitelisted.
PreferenceServer write-window gating and setup-mode wiring `firmware/host/modules/connectivity/preference-server.ts`, `firmware/host/app/setup-mode.ts`	`PreferenceServer` adds `#writesEnabled`/`#writeWindowTimer` state, `enableWrites(durationMs?)`/`disableWrites()` methods, and gates `receiveAndSetPreference` via `validatePreferenceWrite`; setup-mode captures the server instance and calls `enableWrites()`.
Test fakes and module aliasing config `firmware/host/modules/connectivity/__tests__/fakes/uartserver.ts`, `firmware/tsconfig.test.json`, `firmware/host/modules/connectivity/__tests__/preference-write-guard.test.ts`	Adds a fake `UARTServer`/`SERVICE_UUID`, new tsconfig path aliases for `preference-server`, `preference-write-guard`, and `uartserver`, and test scaffolding/module aliasing setup.
Write-guard and PreferenceServer integration tests `firmware/host/modules/connectivity/__tests__/preference-write-guard.test.ts`	Unit tests cover allowed/unknown-domain/writes-disabled validation; integration tests verify write rejection before `enableWrites()`, acceptance after enabling, rejection of non-whitelisted keys, and write-window closure after a duration.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SetupMode
  participant PreferenceServer
  participant WriteGuard as validatePreferenceWrite
  participant Preference

  SetupMode->>PreferenceServer: enableWrites(durationMs)
  PreferenceServer->>PreferenceServer: start write-window timer, set writesEnabled=true
  Note over PreferenceServer: BLE client sends domain.key/value
  PreferenceServer->>WriteGuard: validatePreferenceWrite(allowedKeys, writesEnabled, domain, key)
  WriteGuard-->>PreferenceServer: decision (allowed or rejection reason)
  alt allowed
    PreferenceServer->>Preference: set(domain.key, value)
    PreferenceServer-->>SetupMode: onPreferenceChanged
  else rejected
    PreferenceServer->>PreferenceServer: trace(rejection reason)
  end
  PreferenceServer->>PreferenceServer: write-window timer fires, disableWrites()

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change: guarding BLE preference writes.
Linked Issues check	✅ Passed	The PR adds a write window and PREF_KEYS-based whitelisting, addressing the unauthenticated BLE preference write issue [`#505`].
Out of Scope Changes check	✅ Passed	The test fakes, aliases, and setup-mode wiring are directly supporting the BLE write-guard fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/issue-505-ble-pref-write-guard

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

meganetaaan · 2026-07-08T00:05:22Z

@coderabbitai review

coderabbitai · 2026-07-08T00:05:28Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

firmware/host/app/setup-mode.ts (1)
60-76: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Time-box the BLE write window
firmware/host/app/setup-mode.ts:76 — pass a durationMs to preferenceServer.enableWrites(). The whitelist limits what can change, but the opt-in write window should still expire automatically instead of staying open for the full setup session.
🤖 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 `@firmware/host/app/setup-mode.ts` around lines 60 - 76, The BLE write window
is left open indefinitely because PreferenceServer.enableWrites() is called
without an expiry; update setup-mode.ts to pass a durationMs when enabling
writes so the opt-in window closes automatically after a short period. Use the
existing PreferenceServer instance in setup-mode.ts and adjust the
enableWrites() call to include an appropriate time limit while keeping the
current whitelist behavior intact.

🤖 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 `@firmware/host/app/setup-mode.ts`:
- Around line 60-76: The BLE write window is left open indefinitely because
PreferenceServer.enableWrites() is called without an expiry; update
setup-mode.ts to pass a durationMs when enabling writes so the opt-in window
closes automatically after a short period. Use the existing PreferenceServer
instance in setup-mode.ts and adjust the enableWrites() call to include an
appropriate time limit while keeping the current whitelist behavior intact.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 56099d6d-b46f-4dd1-b20b-038c4da05ab2

📥 Commits

Reviewing files that changed from the base of the PR and between 092e56a and 7dd47f8.

📒 Files selected for processing (6)

firmware/host/app/setup-mode.ts
firmware/host/modules/connectivity/__tests__/fakes/uartserver.ts
firmware/host/modules/connectivity/__tests__/preference-write-guard.test.ts
firmware/host/modules/connectivity/preference-server.ts
firmware/host/modules/connectivity/preference-write-guard.ts
firmware/tsconfig.test.json

fix(connectivity): guard BLE preference writes

7dd47f8

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai Bot reviewed Jul 8, 2026

View reviewed changes

meganetaaan added 2 commits July 8, 2026 11:02

fix(connectivity): limit BLE preference write window

b4c2941

fix(connectivity): add WASM preference write guard stubs

2c6bf8a

meganetaaan mentioned this pull request Sep 30, 2026

fix(ble): bound preference writes to a settings window #717

Draft

9 of 13 tasks

Navigation Menu

Uh oh!

fix(connectivity): guard BLE preference writes - #528

fix(connectivity): guard BLE preference writes#528
meganetaaan wants to merge 3 commits into
developfrom
fix/issue-505-ble-pref-write-guard

meganetaaan commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

meganetaaan commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

meganetaaan commented Jul 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

概要

変更内容

検証

未実施

リリース影響

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

meganetaaan commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

meganetaaan commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading