security(sync): Google OAuth as a public client with PKCE, no secret - #32
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
thinkutils | eb316fc | Jul 20 2026, 03:27 AM |
Removes the client secret entirely rather than replacing the leaked one. A desktop binary cannot keep a secret: anything compiled in is readable by anyone holding the binary. That is not theoretical here -- it is exactly how this project's first client secret ended up public, in the initial commit, and it stayed readable for the life of the repository. Google's guidance for installed apps is a public client with PKCE, which this already used. PKCE is what actually protects the exchange: the verifier is generated per authorisation, never leaves the process, and an intercepted authorisation code is useless without it. The secret was adding nothing an attacker could not read, while creating something that had to be rotated when it leaked. So THINKUTILS_GOOGLE_CLIENT_SECRET is gone, not merely unset, and set_client_secret is no longer called. Only the client ID is supplied at build time, and a client ID is not sensitive. Three tests hold the line, asserted against the source because the property is the ABSENCE of a call and there is no runtime state to inspect. Someone re-adding set_client_secret to fix an auth error would otherwise pass review unnoticed. Writing those tests took three attempts, each failure worth keeping: the comments explaining the rule matched the rule then the assertion strings in the test module matched it too and a scan scoped too aggressively would pass while inspecting nothing So the scan covers production code only, strips comments, and asserts it saw the module body -- a guard that inspects nothing passes for the wrong reason. Docs updated: the setup no longer tells anyone to paste a secret into sync.rs, and says plainly that the secret Google offers for a Desktop app client should be ignored.
vietanhdev
force-pushed
the
feat/oauth-pkce-public-client
branch
from
July 20, 2026 03:24
6ce35a2 to
eb316fc
Compare
vietanhdev
added a commit
that referenced
this pull request
Jul 20, 2026
The concurrency block intended never to cancel on main -- the comment said
so -- and did the opposite:
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
The expression renders to the STRING "false", and a non-empty string is
truthy in that position, so main was cancelled like any other ref. It
failed silently for exactly as long as nobody merged twice in quick
succession.
Four main runs were cancelled during this batch of merges (#36, #37, #39,
#32), each with ZERO jobs recorded -- so those commits have no evidence
they ever built. The runs that were supposed to be the record of what
shipped are the ones that got killed.
Encoding the rule in the concurrency GROUP is unambiguous: on main the SHA
gives every run its own group, so there is nothing to supersede; every
other ref keeps a per-ref group, so a force-push still cancels the old run.
tests/workflow_concurrency.rs guards both halves -- an expression-valued
cancel-in-progress, and a group that lost its per-SHA component (which
with cancel-in-progress: true would cancel main on every push, strictly
worse than the bug it replaced). Mutation-verified: restoring the original
two lines fails both.
118 tests.
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.
Removes the client secret entirely, rather than replacing the leaked one.
Stacked on #30 (oauth2 5.0), which it builds against.
Why no secret at all
A desktop binary cannot keep one. Anything compiled in is readable by anyone holding the binary — not theoretical here: it's exactly how this project's first client secret ended up public, in the initial commit, readable for the life of the repo.
Google's guidance for installed apps is a public client with PKCE, which this flow already used. PKCE is what actually protects the exchange — the verifier is generated per authorisation, never leaves the process, and an intercepted authorisation code is useless without it.
The secret was adding nothing an attacker couldn't read, while creating something that had to be rotated when it leaked. So
THINKUTILS_GOOGLE_CLIENT_SECRETis gone, not merely unset. Only the client ID is supplied at build time, and a client ID isn't sensitive.Tests, and what writing them taught
Three assertions hold the line. They check the source, because the property is the absence of a call — there's no runtime state to inspect, and someone re-adding
set_client_secretto fix an auth error would otherwise pass review unnoticed.It took three attempts, and each failure is worth keeping:
So the scan covers production code only, strips comments, and asserts it saw the module body. A guard that inspects nothing passes for the wrong reason — that's the failure mode I'd been bitten by twice already in this repo's packaging tests.
Docs
Setup no longer tells anyone to paste a secret into , and says plainly that the secret Google offers for a Desktop app client should be ignored.
What you still need to do
Create a new OAuth client (type: Desktop app) and build with:
THINKUTILS_GOOGLE_CLIENT_ID="...apps.googleusercontent.com" npm run tauri buildThe flow gets its first real run then — it can't be exercised without a live client.