feat: embed Laminar project key into release binaries#32
Conversation
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/index.ts">
<violation number="1" location="packages/opencode/src/index.ts:5">
P1: `Telemetry.applyTelemetryKey()` is executed too late to guarantee pre-import env gating. Static imports run before top-level statements, so downstream module initialization can read `LMNR_PROJECT_API_KEY` before this call.</violation>
</file>
<file name="packages/bcode-browser/src/telemetry.ts">
<violation number="1" location="packages/bcode-browser/src/telemetry.ts:24">
P2: Use a presence check for `DO_NOT_TRACK` (`!== undefined`) to match the documented "is set" opt-out gate.</violation>
<violation number="2" location="packages/bcode-browser/src/telemetry.ts:25">
P2: Check for `LMNR_PROJECT_API_KEY` presence (`!== undefined`) instead of truthiness; otherwise an explicitly provided empty value is overwritten by the embedded key.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| declare const BCODE_DEFAULT_LMNR_KEY: string | ||
|
|
||
| export const applyTelemetryKey = () => { | ||
| if (process.env.DO_NOT_TRACK) return |
There was a problem hiding this comment.
P2: Use a presence check for DO_NOT_TRACK (!== undefined) to match the documented "is set" opt-out gate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/bcode-browser/src/telemetry.ts, line 24:
<comment>Use a presence check for `DO_NOT_TRACK` (`!== undefined`) to match the documented "is set" opt-out gate.</comment>
<file context>
@@ -0,0 +1,32 @@
+declare const BCODE_DEFAULT_LMNR_KEY: string
+
+export const applyTelemetryKey = () => {
+ if (process.env.DO_NOT_TRACK) return
+ if (process.env.LMNR_PROJECT_API_KEY) return
+ // `typeof` first: in dev (no Bun `define` substitution) the identifier is
</file context>
There was a problem hiding this comment.
Wonʼt change. The DO_NOT_TRACK convention (consoledonottrack.com) is "set to a non-empty value to opt out" — DO_NOT_TRACK=1 opts out, DO_NOT_TRACK= (empty) does not. The current truthiness check matches that exactly: "" is falsy and falls through; "1" / "true" / "0" (non-empty strings) all opt out. Switching to !== undefined would change the contract so that exporting an empty DO_NOT_TRACK opts out, which contradicts the standard.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
- Move applyTelemetryKey() invocation into the telemetry module itself as an import side effect. ESM evaluates module-import side-effects before any subsequent module-load code in the importing file, so this is unambiguously ordered before the rest of opencode's imports — sidestepping the static-import-hoisting concern. - Use `!== undefined` for LMNR_PROJECT_API_KEY presence check so an explicitly-set empty string is respected as 'no key please' instead of being clobbered by the embedded default. - DO_NOT_TRACK truthiness check is intentional and unchanged: the de-facto standard (consoledonottrack.com) defines opt-out as presence with any non-empty value.
Summary
Wires the build-time injection of the OSS Laminar project key into release binaries, with a runtime gate honoring
DO_NOT_TRACKand any user-suppliedLMNR_PROJECT_API_KEY. No Laminar code yet — just the plumbing that lets a future Laminar-init read the key. Self-builds andbun run devget an empty key baked in, so they never emit.Files
packages/bcode-browser/src/telemetry.ts(new) —Telemetry.applyTelemetryKey(). Three-check gate:DO_NOT_TRACK→ user key → embedded default.typeofguard so dev mode (no Bundefinesubstitution) doesn't ReferenceError.packages/opencode/script/build.ts— addsBCODE_DEFAULT_LMNR_KEYto theBun.builddefineblock.JSON.stringify(process.env.BCODE_DEFAULT_LMNR_KEY ?? "")so unset → empty string literal in the bundle.packages/opencode/src/index.ts— callsapplyTelemetryKey()before any other module-init code runs..github/workflows/release.yml— exposessecrets.LMNR_PROJECT_API_KEY_OSSto the build step asBCODE_DEFAULT_LMNR_KEY.Verification
bun run typecheck(filtered): 5/5 passed.BCODE_DEFAULT_LMNR_KEY=test-key-abc123-fake: key is present in the compiled binary (strings bcode | grep test-key-abc123-fakematches).BCODE_DEFAULT_LMNR_KEYunset: assignment branch is dead-code-eliminated; the function reduces to theDO_NOT_TRACK/ user-key checks plusreturn.bcode --versionsmoke test passes both ways.Action required after merge
Add the secret on the repo before tagging the next release. See PR description in Slack thread for the full opt-out + rotation story.
LMNR_PROJECT_API_KEY_OSS. Value: the ingest-only Laminar project key.strings ./bcode-linux-x64 | grep -c <first-8-chars-of-key>is non-zero on the published artifact.If the secret is unset when CI runs, the workflow still succeeds — the build env just gets an empty
BCODE_DEFAULT_LMNR_KEY, identical to a local build.