logs.gokuls.in

4 pull requests merged across 1 repo

Offline-Protocol/offline-protocol-sdk

The npm publish authenticated with a long-lived NPM_TOKEN repository secret. It now

exchanges this workflow's own OIDC identity for a short-lived, workflow-scoped registry

token, so there is no npm credential to exfiltrate from a build log, replay, or rotate.

The trusted publisher is already configured at the registry against this repository and

the workflow filename release.yml.

The publish itself becomes shorter (npm publish --access public --tag "$NPM_TAG", no

NODE_AUTH_TOKEN, no --provenance). Most of the diff is the part that makes a silent

failure loud, because this workflow runs only on a v* tag and has no PR-time coverage:

its first real execution is a real release, which is exactly how the --provenance 422

reached production in v0.20.1.

StepThe failure it exists to prevent
Node '20''24'The OIDC exchange lives in the npm CLI and needs npm >= 11.5.1. Node 20 ships 10.8.2 and Node 22 LTS still ships 10.9.8; neither carries the code path at all. Node 24 ships 11.17.0
Assert npm >= 11.5.1oidc.js is documented as "intended to never throw": every failure branch returns undefined and publishing continues with whatever .npmrc holds. On an older npm, trusted publishing does not fail, it *never happens*. A future node-version downgrade would reopen that with nothing in the diff to review
Strip _authToken from .npmrcsetup-node writes _authToken=${NODE_AUTH_TOKEN} whenever registry-url is set. It does not block the exchange (see below), but if the exchange ever fails npm falls back to it and dies with a 404 that reads as "no such package". Stripping leaves ENEEDAUTH
Rehearse the exchange on dry runsThe first pre-release coverage this workflow has ever had
Verify provenance from the registryA green publish step proves the tarball uploaded, never that it carried provenance

--provenance is deleted rather than ported, along with the whole

Resolve npm provenance eligibility step. It was resolved from

github.event.repository.visibility because the registry answers HTTP 422 to a

provenance publish from a private source repo. npm now applies that rule itself, by

*declining* before publishing rather than by rejecting, so passing the flag explicitly

would opt back out of npm's check and reinstate the 422.

Related issues

None.

Type of change

  • chore — build, tooling, or dependency changes

Checklist

  • Commits follow Conventional Commits
  • Docs / CHANGELOG.md updated (the 0.24.0### Security subsection, since 0.24.0 is cut but not yet tagged, so this ships *in* it rather than after it; plus CONTRIBUTING.md)
  • cargo fmt / clippy / test / cargo-denynot run: this PR changes no Rust. Only .github/workflows/, CHANGELOG.md and CONTRIBUTING.md are touched
  • No new unsafe; no UDL change, so no binding regeneration

Breaking changes

None for consumers. Two operational notes:

  • The filename release.yml is now load-bearing. npm matches a trusted publisher on

the repository plus the workflow *filename*, so renaming it revokes publishing

silently. Factoring the publish step into a reusable workflow_call workflow breaks it

the same way, because npm validates the *calling* workflow's name. Both are called out

in comments at the publish step.

  • Do not delete the NPM_TOKEN secret until one real trusted publish has succeeded.

It is the rollback path, and per CONTRIBUTING.md force-moving a released tag stopped

being a valid recovery once crates.io publishing landed, so a failed publish costs a

patch version.

Notes for reviewers

Two corrections to the commonly-cited root cause, both verified rather than recalled,

since they are why the strip step is worded the way it is:

1. npm leaves an unresolvable ${VAR} in a config file literal, not empty (tested:

npm config get returns the string ${...}). So the key holds a non-empty *garbage*

credential, not an empty one.

2. In lib/commands/publish.js at v11.17.0, await oidc(...) is line 147 and

getCredentialsByURI is line 149. OIDC runs first and overwrites the token. The

stale line therefore masks failures rather than blocking the exchange, which is the

opposite of what actions/setup-node#1551 and npm/documentation#1960 both claim.

How to validate this before merging (the point of the rehearsal step): dispatch

Release & Publish from this branch with dry_run: true and an explicit version.

publish.js calls oidc() before it branches on --dry-run, so the token request, the

exchange and the registry's verdict on this workflow's identity all really happen; only

the upload is skipped. npm matches on repository and workflow filename not on the ref,

so a branch dispatch answers for the tag before the tag exists. An explicit version is

required, because a dry run without one resolves 0.0.0-dev and prepare-npm.sh's

AUTOLINK_MIN_VERSION gate rejects it before the rehearsal is reached.

Verification already done locally, since CI cannot reach any of this:

  • shellcheck --severity=warning (the repo's CI setting) clean on all four new run:

blocks, extracted out of the YAML.

  • All four blocks executed against mocks with negative controls: the version gate is

exact at the boundary (11.5.1 passes; 11.5.0, 10.9.8 and 10.8.2 fail); the strip step

removes _authToken, keeps registry=, and exits 0 when no .npmrc exists; the

rehearsal is correct across all five branches (exchange OK, npm too old, exchange

rejected, unrelated npm error, and a prepublishOnly packaging failure); the provenance

check is correct on present / appears-on-retry / never-appears.

  • npm ci + tsc --noEmit clean under npm 11.12.1 against the real lockfile, which

de-risks the npm 10 → 11 half of the Node bump. npm pack --dry-run --json keeps the

same shape under npm 11, so scripts/prepare-npm.sh's packlist assertion is unaffected.

  • The Node 20 → 24 half is not verified locally (no Node 24 on this machine). Bumping

the react-native-typecheck job in ci.yml is what makes PRs catch it; that job already

states it mirrors release.yml's Node version, so leaving it on 20 would have falsified

the comment.

Deliberately out of scope, happy to do either as a follow-up:

  • No GitHub Environment gate on the publish job. It would be a second field that has to

match npm's config exactly, and there are no environments configured on this repo today.

  • The publish still runs *after* Create GitHub Release, so a publish failure leaves an

orphan release, as in v0.20.1. Worth fixing, but it changes release semantics and does

not belong in the same PR as the credential swap.

Cuts v0.24.0. Minor rather than patch on surface: rekey_session is a new

method on every binding, SecurityConfig / controlFreshnessEnforced is a new

config section, STALE_CONTROL_FRAME is a new warning type, and two crates

publish for the first time. Nothing was removed and no binding signature changed

shape, so no application build breaks on this one.

Version files

All bumped together, 0.23.00.24.0:

  • Cargo.toml ([workspace.package].version + the ten internal-dependency pins)
  • crates/offline-protocol-sealed/Cargo.toml and crates/offline-protocol-leaf/Cargo.toml

(four more pins that live here, not in the workspace table, because cargo

silently ignores default-features = false on an inherited dependency)

  • Cargo.lock, plus tools/embedded-footprint/Cargo.lock and

tools/mls-interop/Cargo.lock (separate workspaces that CI builds --locked)

  • bindings/python/pyproject.toml, bindings/react-native/package.json + lock
  • THIRD-PARTY-NOTICES.md ×3, SECURITY.md, docs/UPGRADING.md

Changelog audit

[Unreleased] covered ten of the fourteen commits in v0.23.0..HEAD. Added

entries for the four it missed:

PRWhat was missing
#408The BLE framing chapter, its conformance vectors, and the identity assertion that had no spec anywhere
#394ADR 0021 and tools/mls-interop, the phone-vs-device MLS harness
#391Four benchmarks compiling to empty stubs; a fifth timing an error return
#392The MLS seal/open cost per DM, the crypto term the send-path bench excludes

No entries were injected into the shipped [0.23.0] section this time (the

0.22.0 cut's failure mode): the section at HEAD is byte-identical to the tag.

Two stale figures corrected, both the "existing entry made stale by a later

commit in the same range" class:

  • The leaf image is 449.5 KiB, not the 449.1 KiB #406 recorded before #407

landed on top of it. Re-measured with tools/embedded-footprint/measure.sh.

  • docs/README.md called the ADR set "Nineteen decisions"; there are 23.

Recipe corrections found by following it

  • CONTRIBUTING.md named eight internal-dependency versions and omitted

both the four local pins in the dual std/no_std crates and the two tool

lockfiles. Missing either fails the build, and the local pins are new since

the last cut.

  • .github/workflows/release.yml claimed all crate names are pre-reserved

on crates.io. That stopped being true when offline-protocol-sealed and

offline-protocol-leaf were added.

⚠️ Before tagging: two crate names are unreserved

offline-protocol-sealed and offline-protocol-leaf both return 404 on

the crates.io index. Every other name is reserved at 0.0.0, so this release

is the first to spend brand-new-crate budget (1 per 10 min, burst 5). Two names

fits inside the burst, and the publish job is resumable if it does not, but

reserving both before pushing the tag turns a possible incident into a no-op.

The no-op probe in the publish job still names offline-protocol-uniffi as the

last crate published. offline-protocol-leaf is also a graph root, so if

cargo-workspaces orders it after uniffi the annotation could read wrong on a

resumed run. It gates nothing (the comment above it says so), so it is left

alone here.

Verification

Every gate run locally at 0.24.0:

  • cargo clippy --workspace --locked -- -D warnings — clean
  • cargo test --workspace --lib2530 passed, 0 failed
  • cargo fmt --all -- --check — clean
  • RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps — clean
  • no_std: core, sealed, and leaf (bare-metal-rng) on thumbv8m.main-none-eabihf — clean
  • scripts/check-license-consistency.sh — clean
  • npx tsc --noEmit in bindings/react-native — clean
  • Links and anchors in every changed doc resolve, including the new

docs/UPGRADING.md §17 the release header points at

Archive

0.23.0 moves to docs/changelog/0.23.md with its relative links rewritten

(./docs/X../X). Verified lossless: re-extracted from the tag and diffed,

identical modulo link prefixes. Rows added to both archive tables.

Why

Until a device implemented this protocol, both ends of every BLE link were built from this source tree, so the fragment framing could stay an internal agreement between the Rust transport and the platform bridges. A leaf node does not own its radio, which makes its firmware the other end of that link.

wire-format.md gave the sizing (4 KiB chunks, 185-byte fragments) but no wire-level format at all: no header layout, no field order, no reassembly rules. That logic lives in crates/offline-protocol-transport/src/ble.rs, and a firmware author would have had to reverse-engineer it out of that plus the Swift and Kotlin bridges.

Everything from the wire format upward has a spec chapter. The layer where firmware actually meets the phone's radio did not.

What this adds

docs/spec/ble-framing.md, covering both layers firmware needs:

  • The GATT contract — one service, three characteristics, which direction each carries, and the notify/indicate rule.
  • The fragment codec — the ten-byte header (magic | version | id_len | message_id | index | total | data_len | data), explicitly little-endian, the every-write-is-framed rule, and the total_fragments = 1 short-circuit.
  • What a receiver owes — the five refusals in order, plus the two local-policy bounds with shipped values.
  • Sizingmax_fragment_payload = mtu - 10 - id_len, with the real numbers: a UUID message id makes per-fragment overhead 46 bytes, leaving 139 at the floor MTU.

Conformance vectorsble-framing-v1.vectors.json: 4 reassembly cases (including out-of-order and duplicated indices) and 9 refusals covering every check. The fragment format had no vectors at all before this, only round-trip tests, which agree with themselves perfectly.

A constants drift guard — parses the chapter's constants table and compares it to constants.rs. A spec that drifts from the code is worse than no spec: an implementer who follows it ships a device that fails in the field rather than at the document.

Four cross-reference edits — the README's layering section declared *all* transport framing out of scope, which stopped being true; wire-format.md was conflating chunks with fragments under one heading; leaf-provisioning.md left getting frames onto the radio unsaid; transport-architecture.md listed three of four characteristics, omitting the identity one that makes the device id worth reading.

Three things the writing turned up

1. The service UUID is Nordic's UART Service, adopted rather than minted. Documented as-is (changing it is a wire break) with the collision cost named, rather than implying it was chosen.

2. The notify/indicate split is not a divergence to converge. Android advertises INDICATE alone deliberately: notifications are not flow-controlled, so a fast peripheral outruns a slower central and loses one fragment per pass, and a message losing one fragment per pass never reassembles. Worst on the first large message of a pairing. The chapter tells firmware to offer indication alone and says why.

3. The header's message id is an assembly key only — neither authenticated nor cross-checked against the id inside the payload. Benign, but a firmware author would reasonably assume otherwise, so it is now an explicit MUST NOT.

Vectors are computed from the chapter, not the code

Per the convention item 18 established: a vector generated from encode_fragment passes against any format this crate happens to emit, including a wrong one. These were generated from the format definition in the chapter, so a disagreement is evidence about one of them rather than two copies of one mistake agreeing.

Verification

  • cargo test --workspace --lib green; the 4 new tests green; all 15 pre-existing fragment tests still green
  • cargo clippy --workspace --locked -- -D warnings clean (the CI gate; note --all-targets reports 951 pre-existing unwrap_used lints in test code, which ci.yml:139-140 documents as why CI does not lint test targets)
  • cargo fmt --all -- --check clean
  • Every markdown link resolved and the one anchor confirmed against its heading

All four tests were negative-controlled before being trusted — a flipped hex byte in a frame, a drifted constants row, and a wrong sizing example. Each fails its own test and nothing else, so the isolation holds. A guard written for a fix that already exists is vacuous unless you break the fix.

Blast radius

Documentation and new tests only. No production code, no wire change, no binding regeneration (no UDL change), no embedded impact. This documents shipped behaviour rather than altering it. Revert is trivial.

Scope

Contributes to todo item 5 (E2, protocol spec + conformance vectors) rather than closing it. The messaging spec chapters remain the larger half of that item.

A phone could not start a pairing with a leaf

A sender builds the freshness-bound control payload for a recipient that has advertised it and the older one for a recipient that has not. Capabilities travel in a key package, so the first key package to a peer never met is signed under the older payload however new the sender is. ADR 0023 already says this outright, under "First contact necessarily signs v1".

A leaf verified only the freshness-bound payload, on the reasoning that no phone old enough to need the other one had ever paired with a device. That reasoning is about releases, and this is not about releases: today's engine signs the older payload on the frame that opens a phone-initiated pairing, because at that moment it has never seen the device's ctrl_versions.

So the device refused it, and refused every retransmission. The pairing could not start from that side at all: the device learned nothing about the phone, so it had no address to advertise back to, and the phone's ladder delivered ten signature failures to firmware whose only account of itself is that error stream. Only a pairing the device began could complete, and beginning one needs an address it could only have learned from the frame it refused.

The older payload is now accepted on __MLS_KEY_PKG__ and nothing else, which covers both first contact and the case the specification already required, a peer whose record of the device was lost. A frame admitted under it has its session_reset ignored, so the directive that destroys state still needs a stamp inside the signature and #403's replay stays closed; what survives is capability advertisement, which this protocol treats as unauthenticated hint data everywhere else. Its age is not judged, because that payload leaves the timestamp outside the signature.

No durable "this peer proved v2" flag was needed. The specification's held-peer escape is also key-package-only, so both cases collapse into one rule.

Nothing ever scheduled a rekey

ADR 0021 and the leaf provisioning chapter both said post-compromise security "arrives on a cadence the phone sets", and named the rekey interval as what bounds the window. REKEY_INTERVAL_SECS is a floor, and the only caller of schedule_session_rekey is the epoch-desync classifier. A pair that never forks never heals, so the window a stolen key stays useful for was bounded by nothing.

rekey_session drives the existing path deliberately. The teardown and the reset advertisement are now one helper both entry points call, so a peer cannot tell the two apart and they cannot drift. The floor is shared, so a caller looping on this cannot do to a pair what that bound exists to stop an attacker doing. SessionRekeyTriggered is deliberately not shared: it names an epoch desync and exists so a sustained rate of them reads as an attack signature, and a scheduled rotation firing it would hand an operator their own maintenance wearing that shape.

It returns a boolean. false means the window has not lapsed, which is not a failure. The cadence stays the application's because a rotation costs a teardown, a key-package exchange and a re-establish, and what that is worth to a mains-powered lock and to a phone on a metered link are different answers that nothing on the wire distinguishes.

What is covered

15 cases over the real paths, every frame crossing a transport and carrying a peer identity the way a radio reports one:

  • both pairing orders, device-first and discovery-first
  • traffic each way, with the delivery acknowledgement settling the real retry ladder
  • replay in both directions
  • a power cut on the device, and a relaunch of the phone
  • an application-driven rotation, traffic in the epoch it produced, and its own reset frame refused on replay

The device is given no transport. A leaf does not own its radio: it is handed a frame and returns the frames to transmit, so the harness is the firmware.

Verification

  • cargo clippy --workspace -- -D warnings, cargo test --workspace, RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps, cargo fmt --all -- --check
  • All three bare-metal gates (core, sealed, leaf on thumbv8m.main-none-eabihf)
  • tools/mls-interop still passes
  • The 5 new leaf tests are mutation-verified in both directions: two fail against pre-fix source, and the three guards fail against a deliberately broken fix, since a guard written for a fix that already exists is vacuous unless you break the fix
  • All three bindings regenerated together from one bindgen per contract C1, and each bridge checked on its own gate: tsc, the by-hand swiftc -typecheck from BRIDGE_MAINTENANCE.md, and the CI-parity Gradle harness. Each was negative-controlled with a deliberate unresolved reference first, because a harness that silently compiles nothing exits clean.

No wire change, no config field, no new FFI error variant.

Residual

The RAM gate from Stage 3 is still open and unchanged: mls-rs heap-allocates its state so the .bss delta is about zero, and only a running device answers it. That is the hardware spike, not this.