Skip to content

fix(bark): rotate GCM IV per push - #113

Merged
IceCodeNew merged 1 commit into
masterfrom
codex/bark-iv-rotation
Jul 24, 2026
Merged

fix(bark): rotate GCM IV per push#113
IceCodeNew merged 1 commit into
masterfrom
codex/bark-iv-rotation

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • generate a fresh 12-character IV for every AES-GCM encrypted Bark payload
  • send the per-message IV alongside ciphertext, as supported by the Bark notification extension
  • retain the configured IV as the Bark App setup and configuration-validation value
  • update focused tests and encryption documentation

Root cause

The publisher reused BARK_ENCRYPTION_IV for every AES-GCM encryption under the same key. GCM requires nonce uniqueness; reuse breaks its authentication and confidentiality guarantees.

The official Bark client accepts an iv field on each encrypted push and uses it in preference to the IV stored in the App:
https://github.com/Finb/Bark/blob/master/NotificationServiceExtension/Processor/CiphertextProcessor.swift

Scope

This PR only changes IV rotation and its directly related tests and documentation. It does not change the 650-character Bark product limit or add APNs payload-budget behavior.

Validation

  • Bark/config focused tests: 69 passed
  • prek run --all-files
  • full pytest with branch coverage: 1029 passed
  • line coverage: 99.89%
  • branch coverage: 99.60%
  • real encrypted Bark E2E using local ignored configuration:
    • forecast: 3/3 successful rounds
    • briefing: 3/3 successful rounds

All E2E rounds completed with accepted encrypted Bark chunks and exit code 0. No credentials, locations, or generated notification content are included here.

Summary by CodeRabbit

  • New Features

    • Bark encrypted pushes now generate a fresh IV for each message and send it with the encrypted payload.
    • Encryption continues to use AES-GCM with per-message IV handling.
  • Documentation

    • Updated setup instructions, configuration examples, and design notes to describe the new IV behavior and compatibility requirements.
  • Tests

    • Added coverage verifying IV generation and rotation across encrypted messages.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@IceCodeNew, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e5fd2cff-178c-429b-ad49-f9045a62ba6d

📥 Commits

Reviewing files that changed from the base of the PR and between ca2750c and f9408f0.

📒 Files selected for processing (6)
  • README.md
  • docs/design.md
  • docs/notes.md
  • env.example
  • tests/test_bark_publisher.py
  • weather_briefing/delivery/bark_crypto.py
📝 Walkthrough

Walkthrough

Bark AES-GCM encryption now generates a fresh 12-character IV for each message, returns it with the ciphertext, and documents the request-carried IV behavior. Tests verify deterministic encryption and distinct IVs across chunks.

Changes

Bark encryption IV rotation

Layer / File(s) Summary
Generate per-message encryption IVs
weather_briefing/delivery/bark_crypto.py
BarkEncryptor generates a random 12-character IV for each encryption call and returns it with the base64 ciphertext.
Validate rotation and update protocol documentation
tests/test_bark_publisher.py, README.md, docs/design.md, docs/notes.md, env.example
Tests verify deterministic IVs and distinct IVs for separate chunks; documentation describes sending each generated IV with the encrypted push and using it for decryption.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BarkPublisher
  participant BarkEncryptor
  participant BarkApp
  BarkPublisher->>BarkEncryptor: Encrypt message
  BarkEncryptor-->>BarkPublisher: Return ciphertext and fresh IV
  BarkPublisher->>BarkApp: Send encrypted push with IV
  BarkApp->>BarkApp: Decrypt using request IV
Loading

Possibly related PRs

Suggested labels: 🕐 40+ Minutes

🚥 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 matches the main change: rotating the Bark AES-GCM IV for each push.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/bark-iv-rotation

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.

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.83%. Comparing base (0cae1a7) to head (f9408f0).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #113   +/-   ##
=======================================
  Coverage   99.83%   99.83%           
=======================================
  Files          93       93           
  Lines       10509    10517    +8     
  Branches      620      620           
=======================================
+ Hits        10492    10500    +8     
  Misses         12       12           
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Rotate AES-GCM IV per Bark push and send per-message iv field

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Generate a fresh AES-GCM IV for each encrypted Bark payload to prevent nonce reuse.
• Include the per-message iv in the Bark /push JSON alongside ciphertext.
• Update docs, examples, and tests to reflect IV rotation behavior.
Diagram

graph TD
  ENV["Bark env vars"] --> PUB["BarkPublisher"] --> ENC["BarkEncryptor"] --> AES["AESGCM"] --> BARK["Bark API/App"]
  ENC --> RNG["secrets.choice"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Generate IV as raw bytes (os.urandom) and hex/base64 encode
  • ➕ Avoids punctuation/escaping edge cases by encoding bytes into a restricted charset.
  • ➕ Clearer mapping to the AEAD nonce concept (12 random bytes).
  • ➕ Easy to make deterministic in tests by patching the byte generator.
  • ➖ Requires agreeing on an encoding (hex/base64) that Bark definitely treats as ASCII bytes on decode.
  • ➖ If Bark expects the IV string bytes directly (not decoded), encoding reduces entropy per character unless handled carefully.
2. Restrict IV alphabet to Bark docs’ hex format (secrets.token_hex(6))
  • ➕ Matches the documented way users generate IVs (openssl rand -hex 6).
  • ➕ Eliminates risk of client-side handling issues with punctuation.
  • ➖ Lower entropy per character than a full printable ASCII alphabet (though still typically sufficient at 48 bits).
  • ➖ May be seen as unnecessary constraint if Bark truly accepts arbitrary ASCII.

Recommendation: The PR’s approach (per-message IV generation + sending iv alongside ciphertext) is the correct fix for the GCM nonce-reuse bug and aligns with the Bark client behavior of preferring request-provided IVs. The only strategic follow-up worth considering is narrowing the IV character set (e.g., hex) to reduce interoperability risk with punctuation-heavy IVs; otherwise the change is appropriately scoped and minimal.

Files changed (6) +25 / -13

Bug fix (1) +8 / -6
bark_crypto.pyGenerate a fresh AES-GCM IV per encryption call +8/-6

Generate a fresh AES-GCM IV per encryption call

• Stops retaining a single configured IV and instead generates a new 12-character IV for every encryption. Returns the per-message IV alongside the base64 ciphertext+tag so the publisher can send it with each push.

weather_briefing/delivery/bark_crypto.py

Tests (1) +12 / -3
test_bark_publisher.pyMake Bark IV generation deterministic and assert IV rotation +12/-3

Make Bark IV generation deterministic and assert IV rotation

• Patches 'secrets.choice' to produce predictable IVs for stable crypto tests. Updates publisher test expectations from IV reuse to per-chunk IV rotation and asserts 'iv' is surfaced on the encrypted payload.

tests/test_bark_publisher.py

Documentation (3) +3 / -3
README.mdDocument per-push IV rotation for Bark encryption +1/-1

Document per-push IV rotation for Bark encryption

• Clarifies that 'BARK_ENCRYPTION_IV' is used to configure/seed the Bark app, while each encrypted push carries a newly generated IV used for that message.

README.md

design.mdUpdate Bark encryption design notes to reflect IV rotation +1/-1

Update Bark encryption design notes to reflect IV rotation

• Reframes the Bark encryption boundary as an initial/app IV plus per-message generated IV. Documents that the publisher sends the IV with each ciphertext and the app prefers the request IV.

docs/design.md

notes.mdRevise security rationale: GCM requires unique IV per key +1/-1

Revise security rationale: GCM requires unique IV per key

• Updates the encryption notes to explicitly call out GCM IV uniqueness requirements and describe the new behavior: per-message random IV, with configured IV retained for setup/validation.

docs/notes.md

Other (1) +2 / -1
env.exampleAdjust Bark encryption example comments for IV rotation +2/-1

Adjust Bark encryption example comments for IV rotation

• Updates environment documentation to describe the configured IV as an initial value and that each push uses a new IV sent with ciphertext.

env.example

@qodo-code-review

qodo-code-review Bot commented Jul 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 46 rules

Grey Divider


Remediation recommended

1. Monkeypatch patches import site ✓ Resolved 📘 Rule violation ▣ Testability
Description
Tests patch weather_briefing.delivery.bark_crypto.secrets.token_urlsafe, which is an import site
rather than the defining module secrets.token_urlsafe. This makes the tests more brittle to
refactors and violates the patch-targeting guideline.
Code

tests/test_bark_publisher.py[33]

+    monkeypatch.setattr("weather_briefing.delivery.bark_crypto.secrets.token_urlsafe", lambda _: "fixed-iv-123")
Relevance

⭐⭐⭐ High

Team recently accepted changing tests to patch defining modules (not import sites) to reduce
brittleness.

PR-#101
PR-#99
PR-#102

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2274647 requires patches to target the defining module. The changed tests patch
weather_briefing.delivery.bark_crypto.secrets.token_urlsafe, while the production code calls
secrets.token_urlsafe(...) from the imported stdlib module.

Rule 2274647: Patch behavior at its defining module, not where it is imported
tests/test_bark_publisher.py[33-33]
tests/test_bark_publisher.py[172-172]
weather_briefing/delivery/bark_crypto.py[7-7]
weather_briefing/delivery/bark_crypto.py[47-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests currently patch `weather_briefing.delivery.bark_crypto.secrets.token_urlsafe` (an import site). The compliance rule requires patching at the symbol's defining module (`secrets.token_urlsafe`) to reduce brittleness.

## Issue Context
`weather_briefing.delivery.bark_crypto` imports the stdlib `secrets` module and calls `secrets.token_urlsafe(...)`. The test should therefore patch `secrets.token_urlsafe` directly.

## Fix Focus Areas
- tests/test_bark_publisher.py[33-33]
- tests/test_bark_publisher.py[172-172]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@IceCodeNew
IceCodeNew force-pushed the codex/bark-iv-rotation branch 2 times, most recently from 2c7eae8 to 0f18d5b Compare July 24, 2026 09:17
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread tests/test_bark_publisher.py Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 0f18d5b

@IceCodeNew
IceCodeNew force-pushed the codex/bark-iv-rotation branch from 0f18d5b to ada70af Compare July 24, 2026 09:22
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@IceCodeNew
IceCodeNew force-pushed the codex/bark-iv-rotation branch from ada70af to f9408f0 Compare July 24, 2026 09:24
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f9408f0

@IceCodeNew
IceCodeNew merged commit 24f297b into master Jul 24, 2026
18 checks passed
@IceCodeNew
IceCodeNew deleted the codex/bark-iv-rotation branch July 24, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant