Skip to content

fix(security): pin Sparkle and the DuckDB extension, and publish only what is verified - #2348

Merged
datlechin merged 1 commit into
mainfrom
fix/supply-chain
Aug 21, 2026
Merged

fix(security): pin Sparkle and the DuckDB extension, and publish only what is verified#2348
datlechin merged 1 commit into
mainfrom
fix/supply-chain

Conversation

@datlechin

Copy link
Copy Markdown
Member

Five supply-chain items from the CI audit. Every pin here was verified by fetching the artifact and hashing it, not copied from a report.

The signing path installed an unpinned Sparkle

sign-and-appcast.sh ran brew install --cask sparkle and then picked a directory with ls | head -1. That step holds SPARKLE_PRIVATE_KEY, the EdDSA key that signs every update every user receives, so it should not be running a binary whose contents can change between releases.

It now downloads a pinned release tarball and verifies it. Checked by downloading it here:

actual   015336b601493e05c237964954bff6191370003d94edefe663724c88840d73cc
declared 015336b601493e05c237964954bff6191370003d94edefe663724c88840d73cc

The archive has ./bin/ at its root and carries generate_appcast, which is the only binary this script invokes; extracted and run it to be sure. Version 2.9.5 matches the Sparkle framework pinned in Package.resolved, so the tool and the framework move together.

The iOS DuckDB build compiled a moving ref into a shipped binary

duckdb_extension_load(quack
    GIT_URL https://github.com/duckdb/duckdb-quack
    GIT_TAG main)

This ends up in a binary that ships on the App Store, and with a branch ref nobody can say what was in a given release. Pinned to 7e80f7ff, the head of quack's v1.5-variegata line.

One thing this does not fix, recorded in the file rather than hidden. The file's own rule is that each pin should carry a duckdb submodule matching DUCKDB_VERSION, which is v1.5.2. That commit carries v1.5.4, and no commit in the recent history of v1.5-variegata carries v1.5.2, so an exact match is not available on that line. Closing the gap means either moving DUCKDB_VERSION to v1.5.4, which also has to move the bundled macOS libduckdb.a, or backporting quack. Pinned anyway, because a fixed commit on the right release line is strictly better than tracking main, which today is further from v1.5.2 than v1.5.4 is.

The comment also referenced a QUACK_GIT_TAG variable that has never existed. Rewritten to describe what the file actually does.

The published archive carried 80 MB nothing verified

publish-libs.sh built the macOS archive with tar czf ... -C Libs .. The dot swept in Libs/ios, Libs/dylibs and the .downloaded markers, so every download of the macOS libraries also carried the iOS xcframeworks, which that script's checksum guard does not cover at all.

It now publishes an allowlist: exactly the .a files the guard walks, plus checksums.sha256. Measured against the current tree:

size members
before 177 MB 541
after 97 MB 61

61 is the 60 entries in Libs/checksums.sha256 plus the checksum file itself. Verified that the baseline is fully covered and there are no extras.

A personal Apple ID in a tracked file

build-release.sh carried APPLE_ID="${APPLE_ID:-datngoquoc@icloud.com}" in a public repository. It is also dead: notarization goes through --keychain-profile "TablePro" at all three call sites and never reads it.

A secret on the process argument list

The same script passed ANALYTICS_HMAC_SECRET=... to xcodebuild on the command line, where any local process can read it. It is redundant: build.yml writes the value into Configs/Secrets.xcconfig before the build step, and that is how the project reads it. The argv pass is removed.

Predictable temp paths

publish-libs.sh staged a public upload at a fixed /tmp name and check-redis-command-routing.sh used one too, unlike every sibling check script. /tmp is world-writable, so a fixed name is something another local user can create first. Both use mktemp -d with a cleanup trap now.

Verification

shellcheck -x --severity=warning is clean on all four scripts. The Sparkle pin and the archive contents were verified by actually fetching and building them. The iOS DuckDB change cannot be verified here without an iOS DuckDB build, which is why the remaining submodule mismatch is written into the file rather than left implicit.

https://claude.ai/code/session_013MEaba8K1HQcyDNeq5wEFk

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 80e7e97 into main Aug 21, 2026
7 checks passed
@datlechin
datlechin deleted the fix/supply-chain branch August 21, 2026 16:21
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.

1 participant