FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

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

Draft
meganetaaan wants to merge 3 commits into
developfrom
fix/issue-505-ble-pref-write-guard
Draft

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

Conversation

meganetaaan commented Jul 8, 2026
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

概要

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 Bot commented Jul 8, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

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()
Loading 🚥 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.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

coderabbitai Bot commented Jul 8, 2026
edited
Loading

Copy link
Copy Markdown
Contributor
✅ 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 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BLE preference server が無認証で任意のPreference書き込みを受け付ける

1 participant


Back | FazBrowse Home | New Git URL