inject google oauth credentials at build time#154
Conversation
There was a problem hiding this comment.
2 issues found across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
marked as draft - as requires manual steps by @ruzin to be safe for merge |
|
Thanks for moving the OAuth creds out of source — good change. One cross-platform blocker before this can merge though: 🚫 Breaks Google Calendar on Windows buildsThis PR removes the hardcoded So on a Windows release: Fix: add the same secret-injection step to
Minor (non-blocking)
Happy to help wire up the Windows workflow step if useful. |
|
Status check as part of a housekeeping pass - this PR has been sitting since June 11 and now conflicts with One nuance worth stating for the decision: for a desktop app's installed-app OAuth flow, Google's own docs treat the client secret as not confidential - it ships inside every binary regardless of how it gets there. So this is repo hygiene and rotation policy rather than an acute leak. But that cuts both ways: build-time injection (this PR) only keeps it out of the repo, not out of the app. @ruzin I think this deserves an explicit call rather than staying parked:
Happy to help with whichever direction you pick. |
Summary
The Google OAuth client ID and secret are currently hardcoded in
app/main.js:360-361and committed to source. They've been in public git history since whenever they landed — meaning every clone, fork, and scanner-database has them. This PR moves both values to build-time injection via a generatedapp/build-config.jsmodule written by CI from GitHub Actions secrets.What this PR does
app/main.js— replaces the two hardcoded constants with atry { require('./build-config') }lookup at startup. Empty strings on missing file → calendar surfaces as "not connected" rather than crashing.app/build-config.example.js(new) — template module with empty strings + comments for local-dev setup..gitignore—app/build-config.jsis never committed..github/workflows/build-release.yml— new step "Inject build-time secrets" betweennpm ciand the build. Fails fast if either secret is missing; otherwise writesapp/build-config.jsvianode -e+JSON.stringifyso all escaping is handled correctly. electron-builder packages it into the asar at build time.CONTRIBUTING.md— local-dev setup step pointing atbuild-config.example.js.dotenv; we dropped it in favour of a plainrequire()since (a) it's two constants, not a config system, (b) you already do all your other env-var lookups with rawprocess.env(E2E vars at the top ofmain.js), and (c) it matches your "Node primitives + comment explaining why" pattern across the codebase.What this PR does NOT do
Worth saying out loud so we don't oversell: this doesn't prevent someone unpacking the DMG with
npx asar extractand grepping out the credentials. That's an unavoidable property of any OAuth secret shipped in a desktop binary, and Google's docs explicitly accept it for Desktop-application client type ("the client secret is obviously not treated as a secret in this context").What it does change:
For a real fix to the brand-impersonation phishing vector (someone using the real client to spin up a fake "Steno Calendar Sync"), the only viable path is a serverless proxy that holds the secret server-side. That's a separate, larger change — sketched as a follow-up.
Action required to merge
Three steps, all yours @ruzin:⚠️ ⚠️ ⚠️
Rotate the secret in Google Cloud Console. The current value should be treated as compromised (it's been public). If the OAuth client isn't already type "Desktop application," create a new one. Revoke the old client.
Add the new values as GitHub Actions secrets at Settings → Secrets and variables → Actions:
GOOGLE_OAUTH_CLIENT_IDGOOGLE_OAUTH_CLIENT_SECRETMerge. The next tagged release picks up the new values automatically. No code change needed for future rotations — just update the secret variables.
If steps 1–2 aren't done before the next release, the inject step fails the build loudly with a
::error::annotation pointing at exactly where to add the secrets. No silent ship of an empty-credential DMG.Local dev impact
Contributors who skip the setup launch the app normally — calendar surfaces "not connected", everything else works. Setup is
cp build-config.example.js build-config.js+ their own Google Cloud project credentials.CONTRIBUTING.mdwalks through it.Test plan
node --check app/main.jsclean.npm installclean;package-lock.jsonno longer has a directdotenventry (transitive via electron-builder is fine, unchanged).workflow_dispatchbuild and confirm the resulting DMG authenticates to Google Calendar (full end-to-end: install DMG → connect calendar → see events).::error::message instead of silently shipping.build-config.jsand surfaces "calendar not connected" cleanly (no crash).build-config.jspopulated: confirm calendar connect still works.Follow-ups (out of scope)
POSTHOG_API_KEYatmain.js:356is the same kind of leaked credential. Same fix pattern, ~3 lines, intentionally left out of this PR to keep the diff focused.OUTLOOK_CLIENT_IDatmain.js:367is also hardcoded. Microsoft public clients don't use a secret so the risk profile is different, but the externalisation is good hygiene.build-config.js+ workflow pattern works onwindows-latestrunners with no changes — just swapelectron-builder --macfor--winin the build script.git logforever as a fact of git history. Mitigation = having rotated it.git filter-branch/ BFG can rewrite history but breaks every fork and is rarely worth the disruption.