Skip to content

Bump sendspin-cpp pin to 985fba5 (post-v0.9.5) - #226

Open
bbangert wants to merge 1 commit into
mainfrom
chore/sendspin-cpp-bump-985fba5
Open

bbangert wants to merge 1 commit into
mainfrom
chore/sendspin-cpp-bump-985fba5

Conversation

@bbangert

Copy link
Copy Markdown
Owner

Pin

b95cf854a784ed308b1f8fe9697f414b02768ddd (v0.9.5) → 985fba57afb0462b520ab9ce974d1860858ed685 (main, post-v0.9.5, upstream #210)

No tag newer than v0.9.5 exists upstream, so this pins branch HEAD. Three commits land in the range.

Why this matters

The motivating change is upstream #208, which moves platform_time_us() to CLOCK_MONOTONIC_RAW on Linux. std::chrono::steady_clock is CLOCK_MONOTONIC, which NTP slews; aiosendspin already timestamps on the raw clock, so while the host's time daemon corrects the system clock the measured offset moves at the slew rate, outruns the time filter's drift model, and playback hard-corrects. Upstream measured a 31 ms divergence from one systemd-timesyncd sync, and repeated loss of sync without the fix. That is exactly the failure mode our rpi3 players would see on a device whose clock is being disciplined.

#208 also adds SendspinClient::now_us() so consumers can stamp in the library's clock domain — and that is a required change on our side, not an optional one. AlsaPipeSink stamped notify_audio_played() with its own steady_clock; after the bump that reading sits in a different clock domain than the sync task, which would reintroduce the same drift from the consumer side. This PR therefore also changes c_src/sendspin_player/src/alsa_pipe_sink.cpp to call SendspinClient::now_us(). It is one line plus a comment, and the bump is not safe to take without it.

The other two commits are inert for us: #210 adds a Windows clang-cl path (the Linux branch of cmake/host.cmake and network_info.cpp only had includes regrouped and warning flags moved behind a non-MSVC else()), and #206 is a dependabot bump of a release-drafter GitHub Action.

Patch 0003 re-validation

patch -p1 --dry-run against upstream at the new SHA applies cleanly with no fuzz or offset — no regeneration needed. The hook is not absorbed upstream; the markers are absent before the patch and present after. The CMake configure-time assert_patch_markers gate passed on both markers, and sendspin_host_listener_bound resolves in the linked binary.

Host build

Verified in the cloud env (Ubuntu 24.04, cmake 3.28.3, g++ 13.3.0, libasound2-dev): cmake -S c_src/sendspin_player -B /tmp/ssp-build && cmake --build /tmp/ssp-build -j configures and builds to a linked sendspin_player, with no warnings from our four translation units under -Wall -Wextra.

Only CMakeLists.txt and alsa_pipe_sink.cpp change; no priv/ binaries or build output are committed.

Before release

Firmware needs a rebuild and rpi3 hardware validation — the clock change is a timing-path change, so the thing to watch on-device is sync behaviour across an NTP correction, not just that audio plays. No other open PR touches the sendspin-cpp pin.


Generated by Claude Code

Upstream now reads the host clock from CLOCK_MONOTONIC_RAW on Linux so
NTP slew no longer drags the measured time offset across the hard-sync
threshold. Our ALSA sink stamped notify_audio_played() with its own
steady_clock, which after this bump sits in a different clock domain
than the sync task, so it moves to the new SendspinClient::now_us().

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XKijnP8uVFcwYBESpoUeYP

Copy link
Copy Markdown
Owner Author

Hex audit is failing on this head, and it is not this PR's failure.

plug_crypto 2.2.0 - EEF-CVE-2026-105963 (MEDIUM)
  Secret key base exposed in stack trace arguments on wrong-typed Plug.Crypto arguments
::error::hex.audit found non-allow-listed advisories

This PR changes two C++ files (c_src/sendspin_player/CMakeLists.txt, src/alsa_pipe_sink.cpp) and does not touch mix.exs or mix.lock. The job failed because the OSV advisory database changed under us: EEF-CVE-2026-105963 was published after main last ran CI, and the workflow's ALLOWED list carries only the cowlib advisory (EEF-CVE-2026-43966), so the new one trips the grep -Ev guard.

main is not known-green against today's advisory data — its most recent CI run (e380770, 2026-10-21) predates the advisory's publication, so it passed on data that no longer holds. Any PR opened against main right now will fail this check identically until the lockfile moves, so this is a repo-wide blocker rather than something specific to this branch. A re-run would reproduce it exactly — hex.audit is deterministic against a fixed lockfile — so I have not spent one on it.

Proposed patch, not applied here: plug_crypto is a transitive dependency (via phoenix and plug), pinned at 2.2.0 in mix.lock and not declared in mix.exs. Hex has 2.2.1, published 2026-10-23, which from its timing is very likely the fix release — I could not reach osv.dev or the GHSA page from this environment to confirm the patched range, so that should be checked before relying on it.

mix deps.update plug_crypto   # 2.2.0 → 2.2.1, mix.lock only
mix hex.audit                 # confirm the advisory clears

I am deliberately not pushing that here. It is a security-relevant dependency change that belongs in its own PR rather than bundled into a C++ pin bump, and this environment has no Elixir toolchain, so I cannot run mix hex.audit or the test suite to prove the change sound before pushing. If 2.2.1 turns out not to cover the advisory, the fallback is adding EEF-CVE-2026-105963 to the workflow's ALLOWED list with a note — but that accepts the finding rather than fixing it, so the version bump is the better first move.

Everything else on this PR is green or still running; the C++ build and patch re-validation are covered in the description.


Generated by Claude Code

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.

2 participants