fix(privacy): the promises drifted away from the code - #539
Merged
Conversation
A repo-wide audit across six domains. This carries what was small, verifiable, and false today. Every fix ships with a test or a gate, because in every case the defect survived precisely because nothing tested the claim. PRIVACY — two things NETWORK.md promised that were not true. Git commit messages were being sent to a cloud LLM. build_rerank_context_summary put the five most recent commit messages into JudgeRequest.context_summary, which goes verbatim to the configured provider. RerankConfig::enabled defaults to true, so one saved API key switched it on for every analysis cycle, and titles_only did not cover it — that applies to article content. NETWORK.md said "Project files, source code, file contents, and git history never leave your machine." Removed rather than truncated further: a commit subject is prose the developer wrote, and no length makes it not-git-history. The signal it carried is already supplied by dependencies, git-derived work TOPICS and declared interests. New gate scripts/check-privacy-egress.cjs confines raw-content columns to the modules that mine and store them. Six tests: the exact regression, a module nobody has thought of yet, the allowlisted modules, documentation, and the escape hatch in both directions. Wired into validate and CI repo-guards. The disclosure list was also wrong — NETWORK.md claimed OSV package names were the only locally-derived data sent anywhere. npm, PyPI, crates.io, proxy.golang.org and the GitHub advisory API all receive package names; GitHub search gets languages, Reddit subreddits, Stack Overflow tags. Replaced with an exhaustive table, naming the sharpest edge: a private Go module path discloses the org and repo name to Google's module proxy. SECURITY. Any non-empty string in settings.json granted Signal, permanently. key_is_usable returned true for any key not starting with 4DA-, and it is the FAST PATH, so it short-circuited before the validation cache it claimed to rely on was ever consulted. No signature, no network call, no expiry. The comment directly above it asserted the hole was closed; the fix had only covered the 4DA- branch, and there was no test. A Keygen key is now usable only if the cache vouches for THAT key — matching hash, paid tier, inside the freshness window — via the is_cache_valid helper that already existed. Same hole closed in the backup-file layer. Six assertions including a positive control, because a fix that always returned false would strand every real customer. The SSRF guard was switched off by an attacker-controllable string: three sites read `if provider != "ollama"`, and provider is set by the frontend over IPC, so provider "ollama" disabled validation for any base_url including cloud metadata. Replaced with a host-based validate_llm_endpoint, which also fixes the INV-032 false negative — a local LM Studio or llama.cpp is as safe as Ollama and was blocked for not being called "ollama". Private LAN and metadata stay blocked. The secret scanner missed every key format the app accepts: the OpenAI pattern stopped at the first hyphen, so the current Anthropic and OpenAI project formats sailed through while the audit reported 0 findings over 1,958 files. Patterns moved to scripts/lib/secret-patterns.cjs, broadened, plus a placeholder filter keyed on the MATCHED TEXT not the file path — a path exemption would blanket- excuse a real key in a test file. Ten tests, both directions. Broadening it immediately surfaced five live matches, all fixtures, which is how the filter came to exist. The fixtures themselves are assembled from fragments so no grep-able key shape exists in the file. data/*.tmp is now gitignored. The final .json and .bak were; the .tmp they are written through was not, and settings.json.tmp holds the API key and licence key in plaintext for the width of a rename. RELEASE — two config lines that make a release possible at all. bundle.createUpdaterArtifacts was unset and defaults to false, so Tauri emitted no updater signature, latest.json was never produced, and release.yml requires it before publishing. The gate checked that the workflow ASKED for latest.json and never that the bundler could PRODUCE it: text, not meaning. desktop-template.desktop still registered x-scheme-handler/4da four months after the scheme was renamed to fourda, because a custom desktopTemplate opts out of Tauri's generated entry and nothing referenced the file. Also StartupWMClass=4da against a fourda binary, and template variables the schema does not define, which rendered empty. Exec= now passes %U, without which a deep link arrives with no URL. check-release-channel.cjs covers all of it, with four negative tests. Download page: it advertised auto-updates the build cannot perform and explained SmartScreen as "a new app still building reputation — not a warning about the binary". The shipped installer is unsigned; that is a different prompt. Corrected. FRONTEND. Snooze was broken on every card and said so in English. The frontend sends 'snooze' from two call sites; feedbackTypeMap was Record<string, string> with no snooze key, so the lookup yielded undefined, a non-null assertion silenced it, the field was dropped from the payload, and both backend commands rejected. The store then showed a hardcoded-English toast to all 13 locales. Issue #487, still live. Fixed end to end with a real BehaviorAction::Snooze (strength -0.4, half of Dismiss, mirroring the frontend's own -0.05 vs -0.10 — collapsing it into Dismiss would teach ACE that a deferral is a rejection), arms in both commands, and the map retyped to Record<FeedbackAction, string> so omitting a union member is a compile error rather than a runtime drop. The toast reaches i18n through the window handle utils/error-messages.ts already established, with a fallback that still names the failing command — importing the i18n module directly runs .use(initReactI18next) at import time and broke a suite that mocks react-i18next. The crash screen's recovery button was invisible in light theme: white on --color-border, which is #DDDAD2 there, about 1.3:1. Tokenized. BUILD. chacha20 0.10.0 was yanked upstream, so cargo deny fails and takes the Rust job — and therefore Validate Success, the sole required check — down on EVERY open PR. Bumped to 0.10.2, NOTICE regenerated in the same commit because the attribution gate is the only unfiltered leg of that same required check. HOUSEKEEPING. Four i18n scripts and the CI validator iterated a coach namespace no locale directory has ever contained — hence "coach: undefined" in i18n:status. Verified: cargo test --lib 4654 passed 0 failed; pnpm run test 111 files / 1242 passed; typecheck 0 errors; lint 0 errors; audit:public-ready no findings; 23 gate tests green; cargo fmt --check and clippy --lib clean.
This was referenced Aug 28, 2026
Closed
runyourempire
added a commit
that referenced
this pull request
Aug 28, 2026
…he preflight (#545) Today's dry run spent **25 minutes per platform** discovering this: ``` failed to decode secret key: failed to decode base64 secret key: failed to decode base64 key: Invalid symbol 32, offset 0. ``` ASCII 32, at position 0 — `TAURI_SIGNING_PRIVATE_KEY` **begins with a space**. That secret was set on **2026-02-16**, is non-empty, and passed every check this repository has. `gh secret list` shows it present. The preflight's `require` only tests for emptiness. And it had never been exercised at all, because `createUpdaterArtifacts` was unset until #539 — so nothing ever asked Tauri to decode it. The first release that got far enough to need it would have died on it, after a full four-platform compile. ## The change The preflight exists precisely so a credential problem costs **seconds instead of 45 minutes**. It was only checking that credentials are *there*. Now it also checks they are *shaped* like credentials: no leading or trailing whitespace, on every value, on every platform. ## Deliberately narrow It does **not** validate content. That would mean reimplementing base64, minisign and eSigner's auth inside a preflight — and a check that is wrong about a *valid* credential is worse than no check at all, because it blocks a good release. Surrounding whitespace is unambiguous: no credential should ever carry it, and a value pasted with a stray leading space is exactly the failure that just cost an hour of runner time. ## Verified by execution, not by reading The validator was extracted and run against fixtures: | fixture | flagged? | why | |---|---|---| | leading space | ✅ yes | the actual bug | | trailing space | ✅ yes | same class | | leading tab | ✅ yes | same class | | clean base64 key | ✅ no | must not block a good credential | | **empty** | ✅ no | absence is `require`'s job, not this one's | | **password with spaces *inside*** | ✅ no | legitimate; flagging it would break a working release | `bash -n` over the whole extracted preflight passes, and YAML re-parses. That last pair matters. I caught a real bug in my own first attempt at this — the shape loop landed *inside* the `shape()` function because my anchor matched the wrong `esac`. YAML still parsed cleanly, because it is all one block scalar. Only executing the bash found it. ## Not included An `xcrun notarytool history` probe, which would have caught the **other** credential failure this dry run found — Apple rejecting notarization with *"a required agreement is missing or has expired"*, again after a 25-minute macOS build. It is the obvious next step and worth doing. But it cannot be tested from this machine, and the release path is the wrong place to put code I have only read. Recommended as a follow-up rather than shipped blind.
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Findings from a repo-wide audit run today across six domains (release/distribution, security/privacy, scoring/ACE, frontend, data layer, CI/repo hygiene). This PR carries the subset that is small, verifiable, and closes something that is actually false today. The larger items are listed at the bottom rather than silently dropped.
The through-line: the code is careful, and several promises about the code had drifted away from it. Every fix here ships with a test or a gate, because in each case the defect survived precisely because nothing tested the claim.
Privacy — two things NETWORK.md promised that were not true
Git commit messages were being sent to a cloud LLM.
build_rerank_context_summaryput the five most recent commit messages intoJudgeRequest.context_summary, which goes verbatim to whichever provider the user has configured.RerankConfig::enableddefaults totrue, so one saved API key switched it on for every analysis cycle, and thetitles_onlyprivacy setting did not cover it (that applies to article content). NETWORK.md said: "Project files, source code, file contents, and git history never leave your machine."Removed rather than truncated further — a commit subject is prose the developer wrote, and no length makes it not-git-history. The signal it carried is already supplied by dependencies, git-derived work topics, and declared interests: nouns, not the developer's own sentences.
New gate
scripts/check-privacy-egress.cjsconfines raw-content columns to the modules that mine and store them, with six tests covering the exact regression, a module nobody has thought of yet, and the escape hatch in both directions. Wired intovalidateand CI repo-guards. The real defect was that nothing tested the promise.The disclosure list was wrong. NETWORK.md said the only locally-derived data sent anywhere was OSV package names. In fact npm, PyPI, crates.io,
proxy.golang.organd the GitHub advisory API all receive your package names, GitHub search receives your languages, Reddit your subreddits, Stack Overflow your tags. Replaced with an exhaustive table, and the sharpest edge is called out explicitly: a private Go module path such asgithub.com/yourcompany/internal-servicediscloses your organisation and repository name to Google's module proxy.Security
Any non-empty string in
settings.jsongranted Signal, permanently.key_is_usablereturnedtruefor any key not starting with4DA-, and it is the FAST PATH — so it short-circuited before the validation cache it claimed to rely on was ever consulted.{"tier":"signal","license_key":"x"}was a permanent paid tier: no signature, no network call, no expiry. The comment directly above it asserted that this hole was closed; the fix had only ever covered the4DA-branch, and there was no test.Now a Keygen-format key is usable only if the validation cache vouches for that key — matching SHA-256 hash, paid tier, inside the freshness window — via the
is_cache_validhelper that already existed. The same hole in the backup-file recovery layer ("Keygen format — trusted") is closed the same way. Six assertions including a positive control, because a fix that always returnedfalsewould strand every real customer.The SSRF guard was switched off by an attacker-controllable string. Three sites read
if provider != "ollama" { validate_not_internal(url) }.provideris a value the frontend sets over IPC, soprovider: "ollama"disabled validation for anybase_url, cloud metadata endpoints included. Replaced with a host-basedvalidate_llm_endpoint, which fixes the INV-032 false negative in the same change: a local LM Studio, llama.cpp or Jan server is exactly as safe as Ollama and was blocked purely for not being called "ollama". Private LAN and metadata addresses stay blocked — loopback is the user's own machine,169.254.169.254is not.The secret scanner missed every key format the app accepts.
/sk-[a-zA-Z0-9]{32,}/stops at the first hyphen, sosk-ant-api03-...andsk-proj-...both sailed through while the audit reported "0 findings" across 1,958 files. Patterns moved toscripts/lib/secret-patterns.cjs(mirroringpii-hashes.cjs), broadened, and given a placeholder filter keyed on the matched text rather than the file path — a path exemption would blanket-excuse a real key dropped into a test file, which is one of the likelier ways a key leaks. Ten tests, asserting both directions. Broadening it immediately surfaced five live matches, all fixtures, which is how the filter came to exist.Also:
data/*.tmpis now gitignored. The final.jsonand.bakwere; the.tmpthey are written through was not, andsettings.json.tmpholds the API key and licence key in plaintext for the width of a rename.Release — two config lines that make a release possible at all
bundle.createUpdaterArtifactswas unset, and it defaults tofalse. Tauri therefore emitted no updater signature,latest.jsonwas never produced, andrelease.ymlhard-requires it before publishing — so the release stays a draft. The existing gate checked that the workflow asked forlatest.jsonand never that the bundler was configured to produce it: text, not meaning.desktop-template.desktopstill registeredx-scheme-handler/4da, four months after #491 renamed the scheme tofourda, because a customdesktopTemplateopts out of Tauri's generated entry and nothing in the repo referenced the file. AlsoStartupWMClass=4daagainst afourdabinary, and{{exec_dir}}/{{exec_name}}in the desktop action — variables Tauri's template schema does not define, so they rendered empty.Exec=now passes%U, without which a deep link arrives with no URL at all.check-release-channel.cjsnow covers all of it, with four negative tests.Download-page copy. The live page advertises auto-updates the build cannot perform, and explains the SmartScreen prompt as "a new app still building reputation — not a warning about the binary." The shipped installer is unsigned; that is a materially different prompt. Corrected to say so plainly and to point at hash verification.
Frontend
Snooze was broken on every card, and told the user so in English. The frontend sends
'snooze'from two call sites;feedbackTypeMapwas typedRecord<string, string>and had nosnoozekey, so the lookup yieldedundefined, a!silenced it, the field was dropped from the IPC payload, and both backend commands rejected — one on an unknown action type, one on a missing required arg. The store then surfaced a hardcoded-English "Feedback not fully saved" toast to all 13 locales. That is issue #487, still live and louder than when it was filed.Fixed end to end: a real
BehaviorAction::Snooze(strength −0.4, half of Dismiss, mirroring the frontend's own −0.05 vs −0.10 grading — collapsing it into Dismiss would teach ACE that a deferral is a rejection), arms in both commands, and the map retyped toRecord<FeedbackAction, string>so omitting a union member is a compile error rather than a runtime drop. Toast routed through i18n.The crash screen's recovery button was invisible in light theme —
#fffonvar(--color-border), which is#DDDAD2in light: about 1.3:1. Tokenized, along with the wrapper.Housekeeping
Four i18n scripts plus the CI validator iterated a
coachnamespace that no locale directory has ever contained, which is whyi18n:statusprintedcoach: undefined. Removed; the real set is ui / errors / signals.Verification
cargo test --lib— 4654 passed, 0 failedpnpm run test— full frontend suite greenpnpm run typecheck— 0 errorspnpm run audit:public-ready— no findings, re-run after broadening the patternsnode --teston the three touched gate suites — 23 tests green, including 11 new negative testscargo fmt --check,cargo clippy --lib— cleanDeliberately NOT in this PR
Real findings from the same audit that need their own measured change, recorded here so they are not lost:
ace_context.rs:157,monitoring_briefing.rs:894) silently mis-weight an installed build; the scheduled refresh inheritsC:\Windows\System32as its working directory.dependency_edgesis an unbounded append log — 613,674 rows for 12,178 distinct tuples, 145 MB, +9 MB/day, and its only reader is a test. Needs schema 114.office.rsruns two third-party parsers on hostile bytes with nocatch_unwind, wherepdf.rshas one; and no HTTP adapter caps response body size.privacy.llm_content_levelis documented in NETWORK.md but has no UI control and is honoured by 1 of 9 LLM egress sites.