harden(serve): close webhook-secret fail-open + tighten config parsing (OBS-1/2/3)#77
Merged
Conversation
…share int parser Follow-up to the serve+webhooks QA work (PR #76), addressing the three out-of-scope observations the xhigh code-review recorded (OBS-1/2/3). OBS-2 (security): assertTunnelSecurity hardcoded a jira/linear -> secret-env ternary that fell through to null for any other provider, so a future webhook-capable provider could start a public tunnel with no signing secret enforced. Add a single source of truth `WEBHOOK_SECRET_ENV: Record< TrackerProvider, string | null>` plus a `trackerProviderFromEnv()` normalizer in tracker-config.ts; the guard now looks both up. The Record type makes adding a provider a compile error until its secret is declared, and the normalizer derives recognized providers from the map's own keys so it stays in lockstep. OBS-1 (strictness): resolvePollingSyncIntervalMs parsed KANBAN_SYNC_INTERVAL_MS with Number(), accepting hex/scientific notation. Tighten to digits-only + Number.isSafeInteger, matching the strict --sync-interval-ms CLI flag (env path keeps INVALID_CONFIG; CLI path keeps INVALID_ARGUMENT). OBS-3 (dedup): extract `parseBoundedInt` as the single digits-only integer parser; parsePositiveInt, parsePort, and parseSyncIntervalMs all delegate to it, so a future hardening fix lands in one place. Independent codex review: APPROVE. +16 test cases; full suite 491 pass / 27 skip / 0 fail; check clean. Claude-Session: https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad
…ate/enforcement drift) OBS-2 made WEBHOOK_SECRET_ENV the single source of truth for the tunnel gate (assertTunnelSecurity), but the runtime signature enforcement in jira-core/linear-core still hardcoded process.env['JIRA_WEBHOOK_SECRET'] / ['LINEAR_WEBHOOK_SECRET']. The map only guaranteed the gate was exhaustive, not that the gate and enforcement read the same env name — so a future provider whose verification env diverged from its map entry could pass the gate while verification read an unset var, accepting unsigned writes over a public tunnel (fail-open). Route both handlers through a new tracker-config webhookSecretFromEnv() that resolves the secret via WEBHOOK_SECRET_ENV, so gate and enforcement share one source of truth and cannot drift. Also collapses the per-handler double process.env read into a single resolution. Rendered messages are unchanged. +4 tests pinning that the helper reads exactly the env name the map declares and never a sibling provider's env. Suite 495 pass / 0 fail, check clean. Claude-Session: https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad
abpai
added a commit
that referenced
this pull request
Jun 23, 2026
…mitive (#79) Follow-up to #77 (review findings F3 + F2). F3 — JIRA_BOARD_ID used Number.parseInt, which silently coerced trailing garbage and signs into a plausible-but-wrong board id ('12abc'→12, '-5'→-5, '1e3'→1) that was then pinned via Number.isFinite. Now resolved through a strict loader: unset/blank → undefined (no board), set-but-invalid → throw INVALID_CONFIG. This matches how resolvePollingSyncIntervalMs already treats a malformed optional numeric env var (the codebase's own precedent), instead of silently dropping or misinterpreting it. BEHAVIOR CHANGE: a non-empty malformed JIRA_BOARD_ID now fails config loading with INVALID_CONFIG rather than being silently ignored. Updated the wiring test that previously asserted silent fallback. F2 — extracted parseDecimalDigits (transport-input.ts) as the single pure digits-only/safe-integer primitive; parseBoundedInt, resolvePollingSyncIntervalMs, and the new JIRA_BOARD_ID loader all build on it, so "what counts as an integer" has one definition and can't drift (no third hand-rolled copy). Error codes/messages unchanged (INVALID_ARGUMENT vs INVALID_CONFIG preserved per path). +4 tests (parseDecimalDigits unit coverage; JIRA_BOARD_ID accept/omit/reject). Suite 499 pass / 0 fail, bun run check clean. Claude-Session: https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad
This was referenced Jun 23, 2026
abpai
added a commit
that referenced
this pull request
Jun 24, 2026
The serve-hardening and CLI work since v0.7.0 (PRs #76, #77, #78, #79) merged without changesets, so the Release workflow had nothing to consume and the package is still published at 0.7.0. Add the missing changesets so the next release captures this work. - serve-api-webhook-hardening (patch): #76 - envelope webhook-route errors plus tunnel / Postgres-receipt / broadcast / base-path fixes (10 defects). - webhook-secret-fail-open (patch): #77 - single WEBHOOK_SECRET_ENV source of truth for the tunnel gate + stricter KANBAN_SYNC_INTERVAL_MS parsing. - honor-default-task-column (patch): #78 - SQLite createTask honors the configured default task column, with first-column fallback parity with Postgres. - strict-jira-board-id (minor): #79 - a malformed JIRA_BOARD_ID now throws INVALID_CONFIG instead of being silently misparsed (behavior change). Net bump: minor -> 0.8.0. #80 (pure internal refactor) and the README link fix are intentionally omitted as they have no user-facing change. Claude-Session: https://claude.ai/code/session_01X9j5Rs6kXK8BuHguhy3x33
Merged
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.
Summary
Follow-up to the merged serve+webhooks QA work (#76), addressing the three out-of-scope observations its xhigh
/code-reviewrecorded. No behavior change for valid inputs; tightens two validation gaps and removes duplication. Full suite 491 pass / 27 skip / 0 fail (518 ran);bun run checkclean; independent codex review APPROVE.Changes
OBS-2 — close a webhook-secret fail-open (security).
assertTunnelSecurityhardcoded ajira/linear→secret-env ternary that fell through tonullfor any other provider. The day a new webhook-capable provider is added, the tunnel guard would silently start a public tunnel with no signing secret enforced. Now there's a single source of truth —WEBHOOK_SECRET_ENV: Record<TrackerProvider, string | null>plus atrackerProviderFromEnv()normalizer intracker-config.ts— and the guard looks both up:Record<TrackerProvider, …>type makes adding a provider a compile error until its secret env is declared.constructorcan't leak through), keeping it in lockstep with the map.OBS-1 — tighten the env interval parse (strictness).
resolvePollingSyncIntervalMsparsedKANBAN_SYNC_INTERVAL_MSwithNumber(), accepting hex/scientific notation (0x3e8,1e3). Now digits-only +Number.isSafeInteger, matching the strict--sync-interval-msCLI flag. (Env path keepsINVALID_CONFIG; CLI path keepsINVALID_ARGUMENT.)OBS-3 — share one integer parser (dedup). New
parseBoundedIntintransport-input.tsis the single digits-only/^\d+$/+isSafeIntegerparser;parsePositiveInt,parsePort, andparseSyncIntervalMsall delegate to it, so a future hardening fix lands in one place.Verification
bun run checkexit 0.parseBoundedIntbounds/rejection matrix, env hex/scientific/overflow rejection, theWEBHOOK_SECRET_ENVmap contract + normalizer (incl. prototype-pollution guard).parsePositiveInt,sync-config, andassertTunnelSecuritytests pass unchanged (messages/contracts preserved).codexreview: APPROVE, no required changes (the one optional note — making the normalizer derive from the map — is applied here).https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad