harden(serve): close webhook-secret fail-open + tighten config parsing (OBS-1/2/3)

src/__tests__/sync-config.test.ts

@@ -17,7 +17,20 @@ describe('sync config', () => {
   })
 
   test('rejects invalid or too-aggressive intervals', () => {
-    for (const raw of ['999', '5s', '1000.5', '0']) {
+    // OBS-1: digits-only — hex/scientific notation (which Number() would accept)
+    // and over-long digit strings (precision loss / Infinity) are now rejected
+    // for the env var too, matching the strict --sync-interval-ms CLI flag.
+    for (const raw of [
+      '999',
+      '5s',
+      '1000.5',
+      '0',
+      '0x3e8',
+      '1e3',
+      '1_000',
+      '9007199254740993',
+      '9'.repeat(309),
+    ]) {
       let err: unknown
       try {
         resolvePollingSyncIntervalMs(raw)

src/__tests__/tracker-config-webhook-secret.test.ts

@@ -0,0 +1,89 @@
+import { describe, expect, test } from 'bun:test'
+import {
+  WEBHOOK_SECRET_ENV,
+  trackerProviderFromEnv,
+  webhookSecretFromEnv,
+  type TrackerProvider,
+} from '../tracker-config'
+
+// OBS-2: the provider -> webhook-secret-env mapping is a single source of truth.
+// WEBHOOK_SECRET_ENV is typed Record<TrackerProvider, …>, so adding a new
+// provider to the union is a COMPILE error until its secret env is declared here
+// — that compile-time exhaustiveness is the real guard against assertTunnelSecurity
+// silently failing open for a new webhook-capable provider. These runtime tests
+// pin the map contents and the env normalization the guard relies on.
+
+describe('WEBHOOK_SECRET_ENV (single source of truth)', () => {
+  test('maps each known provider to its secret env (local has none)', () => {
+    expect(WEBHOOK_SECRET_ENV).toEqual({
+      local: null,
+      linear: 'LINEAR_WEBHOOK_SECRET',
+      jira: 'JIRA_WEBHOOK_SECRET',
+    })
+  })
+
+  test('every non-local provider points at a *_WEBHOOK_SECRET env name', () => {
+    for (const [provider, envName] of Object.entries(WEBHOOK_SECRET_ENV)) {
+      if (provider === 'local') {
+        expect(envName).toBeNull()
+      } else {
+        expect(envName).toMatch(/_WEBHOOK_SECRET$/)
+      }
+    }
+  })
+})
+
+describe('trackerProviderFromEnv', () => {
+  test('normalizes known providers (case/whitespace-insensitive)', () => {
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: 'jira' })).toBe('jira')
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: 'Linear' })).toBe('linear')
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: '  JIRA  ' })).toBe('jira')
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: 'local' })).toBe('local')
+  })
+
+  test('falls back to local for unset / blank / unrecognized values', () => {
+    expect(trackerProviderFromEnv({})).toBe('local')
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: '' })).toBe('local')
+    // A future provider not yet wired in resolves to local (no webhooks) rather
+    // than an unmapped value; once added to TrackerProvider it must gain a
+    // WEBHOOK_SECRET_ENV entry (compile-enforced).
+    expect(trackerProviderFromEnv({ KANBAN_PROVIDER: 'github' })).toBe('local')
+  })
+
+  test('inherited Object property names do not leak through the map lookup', () => {
+    // The normalizer derives from WEBHOOK_SECRET_ENV's OWN keys, so prototype
+    // names must not be treated as providers.
+    for (const name of ['constructor', 'toString', 'hasOwnProperty', '__proto__']) {
+      expect(trackerProviderFromEnv({ KANBAN_PROVIDER: name })).toBe('local')
+    }
+  })
+})
+
+describe('webhookSecretFromEnv', () => {
+  // The runtime signature enforcement in jira-core/linear-core resolves its secret
+  // through this helper, and the assertTunnelSecurity tunnel gate resolves the same
+  // env name through WEBHOOK_SECRET_ENV. These tests pin that they read the SAME env
+  // name, so the gate and enforcement can't drift into a fail-open.
+  test('reads exactly the env name WEBHOOK_SECRET_ENV declares for each provider', () => {
+    for (const [provider, envName] of Object.entries(WEBHOOK_SECRET_ENV)) {
+      if (envName === null) continue
+      const secret = `secret-for-${envName}`
+      expect(webhookSecretFromEnv(provider as TrackerProvider, { [envName]: secret })).toBe(secret)
+    }
+  })
+
+  test('returns undefined when the declared secret env is unset (open dev mode)', () => {
+    expect(webhookSecretFromEnv('jira', {})).toBeUndefined()
+    expect(webhookSecretFromEnv('linear', {})).toBeUndefined()
+  })
+
+  test('local has no webhook secret env, so it never resolves a secret', () => {
+    expect(webhookSecretFromEnv('local', { JIRA_WEBHOOK_SECRET: 'x' })).toBeUndefined()
+  })
+
+  test('does not read a different env than the one the provider declares', () => {
+    // Setting Linear's secret must not satisfy Jira's lookup, and vice versa.
+    expect(webhookSecretFromEnv('jira', { LINEAR_WEBHOOK_SECRET: 'l' })).toBeUndefined()
+    expect(webhookSecretFromEnv('linear', { JIRA_WEBHOOK_SECRET: 'j' })).toBeUndefined()
+  })
+})

src/__tests__/transport-input.test.ts

@@ -1,5 +1,43 @@
 import { describe, expect, test } from 'bun:test'
-import { parsePositiveInt } from '../transport-input'
+import { ErrorCode, KanbanError } from '../errors'
+import { parseBoundedInt, parsePositiveInt } from '../transport-input'
+
+describe('parseBoundedInt', () => {
+  test('accepts in-range digit strings (inclusive bounds)', () => {
+    expect(parseBoundedInt('0', { min: 0, max: 65535, field: 'port' })).toBe(0)
+    expect(parseBoundedInt('65535', { min: 0, max: 65535, field: 'port' })).toBe(65535)
+    expect(parseBoundedInt('1000', { min: 1000, field: 'iv' })).toBe(1000)
+    expect(parseBoundedInt('  42  ', { min: 1, max: 100, field: 'n' })).toBe(42)
+  })
+
+  test.each([
+    ['65536', { min: 0, max: 65535, field: 'port' }],
+    ['999', { min: 1000, field: 'iv' }],
+    ['-1', { min: 0, max: 65535, field: 'port' }],
+    ['3.5', { min: 0, max: 65535, field: 'port' }],
+    ['0x10', { min: 0, max: 65535, field: 'port' }],
+    ['1e3', { min: 1000, field: 'iv' }],
+    ['', { min: 0, max: 65535, field: 'port' }],
+    ['9007199254740993', { min: 1000, field: 'iv' }], // past MAX_SAFE_INTEGER → precision loss
+    ['9'.repeat(309), { min: 1000, field: 'iv' }], // Number() → Infinity
+  ] as const)('rejects %p as INVALID_ARGUMENT', (value, opts) => {
+    expect(() => parseBoundedInt(value, opts)).toThrow(KanbanError)
+    try {
+      parseBoundedInt(value, opts)
+    } catch (err) {
+      expect((err as KanbanError).code).toBe(ErrorCode.INVALID_ARGUMENT)
+    }
+  })
+
+  test('message form depends on whether max is bounded', () => {
+    expect(() => parseBoundedInt('x', { min: 0, max: 65535, field: 'port' })).toThrow(
+      /port must be an integer between 0 and 65535/,
+    )
+    expect(() => parseBoundedInt('x', { min: 1000, field: '--sync-interval-ms' })).toThrow(
+      /--sync-interval-ms must be an integer >= 1000/,
+    )
+  })
+})
 
 describe('parsePositiveInt', () => {
   test('returns undefined for absent values', () => {

src/index.ts

@@ -9,11 +9,11 @@ import { boardInit, boardReset } from './commands/board'
 import { columnAdd, columnDelete, columnList, columnRename, columnReorder } from './commands/column'
 import { bulkClearDoneCmd, bulkMoveAllCmd } from './commands/bulk'
 import { getConfigPath, loadConfig, saveConfig } from './config'
-import { parsePositiveInt } from './transport-input'
+import { parseBoundedInt, parsePositiveInt } from './transport-input'
 import type { CliOutput, Priority, ProviderCapabilities } from './types'
 import { unsupportedOperation } from './providers/errors'
 import { openKanbanRuntime } from './provider-runtime'
-import { trackerConfigFromEnv } from './tracker-config'
+import { WEBHOOK_SECRET_ENV, trackerConfigFromEnv, trackerProviderFromEnv } from './tracker-config'
 import type { KanbanProvider } from './providers/types'
 import { MIN_POLLING_SYNC_INTERVAL_MS } from './sync-config'
 import { normalizeCreateTaskInput } from './use-cases'
@@ -511,44 +511,22 @@ export function parseServeArgs(argv: string[]): ServeOptions {
   }
 }
 
+// Strict digits-only CLI contract via the shared parseBoundedInt (rejects
+// hex/scientific, the Number() overflow/precision-loss of an over-long digit
+// string, and values below the minimum — all as INVALID_ARGUMENT). The shared
+// resolvePollingSyncIntervalMs (sync-config.ts) is the env-path parser; this
+// flag is validated strictly here.
 function parseSyncIntervalMs(raw: string): number {
-  // Strict digits-only CLI contract (matching --port): reject non-digit input,
-  // the Number() overflow/precision-loss of an over-long digit string
-  // (isSafeInteger also excludes Infinity and values past MAX_SAFE_INTEGER), and
-  // values below the minimum — all as INVALID_ARGUMENT. The shared
-  // resolvePollingSyncIntervalMs (sync-config.ts) is intentionally more lenient
-  // (Number() accepts hex/scientific; isInteger accepts past MAX_SAFE_INTEGER)
-  // and reports INVALID_CONFIG, so validate here and return the parsed value
-  // directly rather than re-parsing the same string through it.
-  const trimmed = raw.trim()
-  const parsed = Number(trimmed)
-  if (
-    !/^\d+$/.test(trimmed) ||
-    !Number.isSafeInteger(parsed) ||
-    parsed < MIN_POLLING_SYNC_INTERVAL_MS
-  ) {
-    throw new KanbanError(
-      ErrorCode.INVALID_ARGUMENT,
-      `--sync-interval-ms must be an integer >= ${MIN_POLLING_SYNC_INTERVAL_MS}`,
-    )
-  }
-  return parsed
+  return parseBoundedInt(raw, { min: MIN_POLLING_SYNC_INTERVAL_MS, field: '--sync-interval-ms' })
 }
 
 // `parseInt` silently accepts `123abc` (→123) and `-1`, and yields NaN for
-// non-numeric input, so validate the port explicitly: digits only, 0–65535
-// (0 lets the OS pick an ephemeral port). Throws so a bad value surfaces as the
-// structured INVALID_ARGUMENT envelope rather than booting on a garbage port.
+// non-numeric input, so validate the port explicitly via parseBoundedInt: digits
+// only, 0–65535 (0 lets the OS pick an ephemeral port). Throws so a bad value
+// surfaces as the structured INVALID_ARGUMENT envelope rather than booting on a
+// garbage port.
 function parsePort(raw: string, label: string): number {
-  const trimmed = raw.trim()
-  const port = Number(trimmed)
-  if (!/^\d+$/.test(trimmed) || port > 65535) {
-    throw new KanbanError(
-      ErrorCode.INVALID_ARGUMENT,
-      `${label} must be an integer between 0 and 65535`,
-    )
-  }
-  return port
+  return parseBoundedInt(raw, { min: 0, max: 65535, field: label })
 }
 
 /**
@@ -573,13 +551,12 @@ export function assertTunnelSecurity(
         'Set KANBAN_API_TOKEN or pass --token <token>.',
     )
   }
-  const providerType = (env['KANBAN_PROVIDER'] ?? 'local').trim().toLowerCase()
-  const webhookSecretEnv =
-    providerType === 'jira'
-      ? 'JIRA_WEBHOOK_SECRET'
-      : providerType === 'linear'
-        ? 'LINEAR_WEBHOOK_SECRET'
-        : null
+  const providerType = trackerProviderFromEnv(env)
+  // Single source of truth (WEBHOOK_SECRET_ENV is Record<TrackerProvider,…>), so
+  // a new webhook-capable provider can't be added without declaring its secret
+  // here — closing the fail-open hole where an unmapped provider would start a
+  // public tunnel with no signature secret enforced.
+  const webhookSecretEnv = WEBHOOK_SECRET_ENV[providerType]
   if (webhookSecretEnv && !env[webhookSecretEnv]) {
     throw new KanbanError(
       ErrorCode.INVALID_ARGUMENT,

src/providers/jira-core.ts

@@ -47,6 +47,7 @@ import type {
   UpdateTaskInput,
 } from './types'
 import { DEFAULT_POLLING_SYNC_INTERVAL_MS } from '../sync-config'
+import { WEBHOOK_SECRET_ENV, webhookSecretFromEnv } from '../tracker-config'
 import { warnOnce } from './warn-once'
 import { applyTaskFilters, forEachWithConcurrency, SyncGate, syncStatusFromMeta } from './sync-core'
 
@@ -872,14 +873,19 @@ export class JiraProviderCore implements KanbanProvider {
   // Shared webhook dispatch. Postgres wraps this with webhook-event auditing;
   // SQLite calls it directly via the default handleWebhook above.
   protected async handleWebhookCore(payload: WebhookRequest): Promise<WebhookResult> {
-    if (!process.env['JIRA_WEBHOOK_SECRET']) {
+    // Resolve the signing secret through WEBHOOK_SECRET_ENV so the runtime
+    // enforcement here and the assertTunnelSecurity tunnel gate read the same env
+    // name — they can't drift into a fail-open where the gate requires one env and
+    // verification reads another (unset) one.
+    const secret = webhookSecretFromEnv('jira')
+    if (!secret) {
       warnOnce(
         'jira-webhook-open-dev-mode',
-        '[jira] JIRA_WEBHOOK_SECRET is not set — accepting webhook without signature verification (open dev mode)',
+        `[jira] ${WEBHOOK_SECRET_ENV['jira']} is not set — accepting webhook without signature verification (open dev mode)`,
       )
     }
     const auth = authorizeWebhook({
-      secret: process.env['JIRA_WEBHOOK_SECRET'],
+      secret,
       rawBody: payload.rawBody,
       signature: headerLower(payload.headers, 'x-hub-signature'),
       verify: verifySha256HmacSignatureHeader,

src/providers/linear-core.ts

@@ -40,6 +40,7 @@ import type {
   UpdateTaskInput,
 } from './types'
 import { DEFAULT_POLLING_SYNC_INTERVAL_MS } from '../sync-config'
+import { WEBHOOK_SECRET_ENV, webhookSecretFromEnv } from '../tracker-config'
 import { warnOnce } from './warn-once'
 import {
   applyTaskFilters,
@@ -596,14 +597,19 @@ export class LinearProviderCore implements KanbanProvider {
   // Shared webhook dispatch. Postgres wraps this with webhook-event auditing;
   // SQLite calls it directly via the default handleWebhook above.
   protected async handleWebhookCore(payload: WebhookRequest): Promise<WebhookResult> {
-    if (!process.env['LINEAR_WEBHOOK_SECRET']) {
+    // Resolve the signing secret through WEBHOOK_SECRET_ENV so the runtime
+    // enforcement here and the assertTunnelSecurity tunnel gate read the same env
+    // name — they can't drift into a fail-open where the gate requires one env and
+    // verification reads another (unset) one.
+    const secret = webhookSecretFromEnv('linear')
+    if (!secret) {
       warnOnce(
         'linear-webhook-open-dev-mode',
-        '[linear] LINEAR_WEBHOOK_SECRET is not set — accepting webhook without signature verification (open dev mode)',
+        `[linear] ${WEBHOOK_SECRET_ENV['linear']} is not set — accepting webhook without signature verification (open dev mode)`,
       )
     }
     const auth = authorizeWebhook({
-      secret: process.env['LINEAR_WEBHOOK_SECRET'],
+      secret,
       rawBody: payload.rawBody,
       signature: headerLower(payload.headers, 'linear-signature'),
       verify: verifyHmacSha256,

src/sync-config.ts

@@ -10,8 +10,12 @@ export function resolvePollingSyncIntervalMs(
   const value = raw?.trim()
   if (!value) return DEFAULT_POLLING_SYNC_INTERVAL_MS
 
-  const parsed = Number(value)
-  if (!Number.isInteger(parsed) || parsed < MIN_POLLING_SYNC_INTERVAL_MS) {
+  // Plain decimal digits only: reject hex/scientific notation (`0x3e8`, `1e3`)
+  // that `Number()` would otherwise coerce, and use `isSafeInteger` so an
+  // over-long digit string rounded past MAX_SAFE_INTEGER / overflowed to
+  // Infinity is rejected rather than silently accepted with precision loss.
+  const parsed = /^\d+$/.test(value) ? Number(value) : NaN
+  if (!Number.isSafeInteger(parsed) || parsed < MIN_POLLING_SYNC_INTERVAL_MS) {
     throw new KanbanError(
       ErrorCode.INVALID_CONFIG,
       `${opts.label ?? 'KANBAN_SYNC_INTERVAL_MS'} must be an integer >= ${MIN_POLLING_SYNC_INTERVAL_MS}`,

src/tracker-config.ts

@@ -4,6 +4,56 @@ import { resolvePollingSyncIntervalMs } from './sync-config'
 
 export type TrackerProvider = 'local' | 'linear' | 'jira'
 
+/**
+ * Single source of truth for which env var holds each provider's webhook signing
+ * secret (`null` = the provider has no webhook ingestion). Typed as
+ * `Record<TrackerProvider, …>`, so adding a new provider to the union is a
+ * compile error here until its secret env is declared — preventing a new
+ * webhook-capable provider from silently slipping past the tunnel-security gate
+ * (assertTunnelSecurity) with no secret enforced.
+ */
+export const WEBHOOK_SECRET_ENV: Record<TrackerProvider, string | null> = {
+  local: null,
+  linear: 'LINEAR_WEBHOOK_SECRET',
+  jira: 'JIRA_WEBHOOK_SECRET',
+}
+
+/**
+ * Normalize the raw `KANBAN_PROVIDER` env value to a known `TrackerProvider`,
+ * falling back to `'local'` for anything unset/unrecognized — matching how
+ * trackerConfigFromEnv resolves the provider. Centralized so the security gate
+ * and the config loader agree on a single derivation.
+ */
+export function trackerProviderFromEnv(
+  env: Record<string, string | undefined> = process.env,
+): TrackerProvider {
+  const raw = (env['KANBAN_PROVIDER'] ?? 'local').trim().toLowerCase()
+  // Recognize exactly the providers declared in WEBHOOK_SECRET_ENV — own keys
+  // only, so inherited names like "constructor"/"toString" don't match — and
+  // fall back to local otherwise. Deriving from the map keeps this normalizer in
+  // lockstep with it, so a newly-added provider is picked up here automatically.
+  return Object.prototype.hasOwnProperty.call(WEBHOOK_SECRET_ENV, raw)
+    ? (raw as TrackerProvider)
+    : 'local'
+}
+
+/**
+ * Resolve a provider's webhook signing secret from `env` via WEBHOOK_SECRET_ENV
+ * (the single source of truth), returning `undefined` when the provider declares
+ * no secret env or the env var is unset — the case where the webhook handler
+ * falls back to open dev mode. The runtime signature enforcement (the provider
+ * webhook handlers) and the assertTunnelSecurity tunnel gate both resolve the
+ * secret env name through this same map, so the gate cannot require one env name
+ * while enforcement reads another (a fail-open if they drifted).
+ */
+export function webhookSecretFromEnv(
+  provider: TrackerProvider,
+  env: Record<string, string | undefined> = process.env,
+): string | undefined {
+  const envName = WEBHOOK_SECRET_ENV[provider]
+  return envName ? env[envName] : undefined
+}
+
 export interface LocalTrackerConfig {
   provider: 'local'
   defaultColumns?: string[]
@@ -34,7 +84,7 @@ export type TrackerConfig = LocalTrackerConfig | LinearTrackerConfig | JiraTrack
 export function trackerConfigFromEnv(
   env: Record<string, string | undefined> = process.env,
 ): TrackerConfig {
-  const provider = (env['KANBAN_PROVIDER'] ?? 'local').trim().toLowerCase()
+  const provider = trackerProviderFromEnv(env)
 
   if (provider === 'linear') {
     const apiKey = env['LINEAR_API_KEY']