Skip to content

fix(security): encrypt channel_config bot_token at rest (#319) - #327

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-319-encrypt-channel-tokens
Apr 16, 2026
Merged

fix(security): encrypt channel_config bot_token at rest (#319)#327
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/issue-319-encrypt-channel-tokens

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Close the MEDIUM plaintext-at-rest finding from #319. workspace_channels.channel_config stored Telegram bot tokens in plaintext JSONB; a DB backup / read-replica mis-grant / ORM query log would have surrendered the bot.

Implements Option B from the triage: field-level AES-256-GCM on bot_token and webhook_secret, with a lazy-migration ec1: prefix so existing plaintext rows keep working and upgrade lazily on the next PATCH /channels/:id or Create.

Changes

  • platform/internal/channels/secret.go — new EncryptSensitiveFields / DecryptSensitiveFields helpers. Idempotent, nil-safe, dev-fallback to plaintext when no key is configured (matches workspace_secrets).
  • platform/internal/handlers/channels.go — encrypt on Create/Update, decrypt before the List masking and the Webhook secret compare.
  • platform/internal/channels/manager.go — decrypt in Reload + loadChannel; rewrite PausePollersForToken to match on decrypted plaintext since channel_config->>'bot_token' = $1 can no longer find encrypted rows.
  • platform/internal/channels/secret_test.go — 6 unit tests (round-trip, idempotent re-encrypt, legacy passthrough, dev no-key fallback, edge cases).

No schema migration required. chat_id and other non-secret fields stay in cleartext so the webhook receiver's channel_config->>'chat_id' lookup stays efficient.

Test plan

  • go test -race ./... across all platform packages — all green (channels 2.478s, handlers 3.199s, middleware 1.583s, wsauth cached)
  • New TestEncryptSensitiveFields_* suite covers round-trip, idempotent, legacy plaintext passthrough, dev no-key, edge cases, nil-safety
  • CI green before merge
  • Cross-vendor review before merge (security-scope)
  • One live round-trip against a Telegram bot after merge (operator smoke test) — recommend resaving the existing channel row to exercise the encrypt path

Closes #319

… rest (#319)

Severity MEDIUM. workspace_channels.channel_config stored Telegram bot
tokens in plaintext JSONB while workspace_secrets encrypts analogous
values with AES-256-GCM. A DB backup leak, read-replica mis-grant, or
any ORM query logging could hand the Telegram bot to an attacker.

Fix: lazy field-level encryption with a version prefix.

  plaintext    "123456:AA..."                (legacy / pre-#319 row)
  ciphertext   "ec1:<base64-GCM-ciphertext>" (new write)

Reads detect the prefix and skip decrypt on legacy rows, so existing
deployments don't need a one-shot data migration — channels upgrade
lazily on the next PATCH/Create (or operators can force-resave).

Only bot_token and webhook_secret are encrypted; chat_id and other
non-secret fields stay in cleartext so the webhook receiver's
channel_config->>'chat_id' lookup stays efficient.

Changes:
- platform/internal/channels/secret.go: new EncryptSensitiveFields /
  DecryptSensitiveFields helpers with the ec1 prefix scheme. Idempotent
  re-encrypt, nil-safe, falls through to plaintext when the dev server
  boots without SECRETS_ENCRYPTION_KEY (matches workspace_secrets' dev
  behaviour).
- platform/internal/handlers/channels.go: encrypt on Create/Update,
  decrypt on List before masking (so the first-4 / last-4 mask operates
  on the real token), decrypt in Webhook before verifying webhook_secret.
- platform/internal/channels/manager.go:
  - Reload and loadChannel decrypt before pushing config into adapters.
  - PausePollersForToken no longer uses channel_config->>'bot_token' = $1
    (can't match ciphertext). Loads all enabled channels and matches on
    decrypted plaintext — small cardinality, negligible overhead.
- platform/internal/channels/secret_test.go: 6 unit tests — round-trip,
  idempotent re-encrypt, legacy plaintext pass-through, dev-no-key
  fallback, empty/non-string/unrelated field skipping, nil-config.

Closes #319

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

PM Review — CI green ✅, ready for security audit sign-off

CI fully green (all 6 checks pass; prior E2E cancellation was a run-supersession artefact, latest run shows overall success).

Implementation looks sound on a quick read:

  • secret.go — AES-256-GCM helpers, idempotent ec1: prefix, nil-safe, dev-fallback when key unconfigured (consistent pattern with workspace_secrets)
  • channels.go — encrypt on Create/Update, decrypt before List masking
  • Lazy migration on PATCH is the right tradeoff for zero-downtime rollout

Flagging for Security Auditor: this directly closes issue #319. Please confirm key-management threat model (env var injection, key rotation story) and approve so we can merge. This PR has no formal review yet.

@HongmingWang-Rabbit HongmingWang-Rabbit left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dev Lead Code Review — ✅ Clean, approved for merge

Issue: #319 — bot_token plaintext in workspace_channels.channel_config

What I checked

  • Write path (handlers/channels.go): EncryptSensitiveFields called in both Create and Update before json.Marshal → DB insert. Plaintext never reaches disk. ✅
  • Read paths (manager.go): DecryptSensitiveFields applied at all four read sites — Reload, loadChannel, PausePollersForToken, and the webhook scan loop. ✅
  • API response masking (handlers/channels.go List): decrypt-before-mask ensures the first-4/last-4 display shows the real token chars, not base64 prefix garbage. ✅
  • Versioning (ec1: prefix): clean sentinel — no real Telegram token starts with ec1:. Lazy upgrade strategy is sound for low-cardinality rows. ✅
  • Dev fallback: crypto.IsEnabled() guard keeps local setups functional without SECRETS_ENCRYPTION_KEY. ✅
  • Tests: round-trip, idempotent double-encrypt, and legacy-plaintext passthrough all covered. ✅

Minor notes (non-blocking — open follow-ups separately)

  1. webhook_secret not masked in List response — pre-existing gap, not introduced by this PR. Recommend a follow-up issue.
  2. Mutex held during crypto in PausePollersForToken — negligible for ≤10 channels; flag if cardinality grows.

Verdict: Implementation is consistent with the workspace_secrets AES-256-GCM posture. Merge once CI completes.

@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit d85ee97 into main Apr 16, 2026
13 of 14 checks passed
HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 16, 2026
Severity MEDIUM. Follow-up to #319/#327. After encrypting bot_token in
channel_config, PausePollersForToken was rewritten to fetch every
enabled channel across all workspaces and decrypt each in Go (since
`channel_config->>'bot_token' = $1` can no longer match ciphertext).
That put every tenant's plaintext token in the Go process's memory on
every discovery call — a blast-radius problem if a heap dump, profiler
endpoint, or future core-dump path ever leaked process memory.

Fix: scope the lookup to the requesting workspace. Discover handler
now requires workspace_id in the request body and passes it through
PausePollersForToken(workspaceID, botToken). Only the caller's own
channels are ever decrypted.

Changes:
- platform/internal/channels/manager.go — PausePollersForToken(ws, tok);
  SQL predicate now `workspace_id = $1 AND enabled = true`. Reload()
  keeps the unscoped query (it legitimately starts every workspace's
  pollers at platform boot).
- platform/internal/handlers/channels.go — Discover body struct gains
  workspace_id field, rejected with 400 if missing, passed to
  PausePollersForToken.
- canvas/src/components/tabs/ChannelsTab.tsx — send workspace_id in the
  discover request body (the component already holds it as a prop).
- platform/internal/handlers/channels_test.go — add
  TestChannelHandler_Discover_329_RequiresWorkspaceID; update the two
  existing Discover tests to include workspace_id so they exercise
  their actual assertion (unsupported-type / invalid-token) instead of
  bouncing at the new scope gate.

Closes #329

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 16, 2026
Wraps up a ~100-tick autonomous triage session by converting the prior
operator's institutional knowledge into standing, checked-in artifacts
so the next team picking up the hourly PR + issue cycle can drop in
without re-discovering everything from scratch.

## New role: Triage Operator

Peer to Dev Lead, Research Lead, Documentation Specialist under PM.
Owns the 7-gate PR verification + issue-pickup cycle across both
molecule-monorepo and molecule-controlplane. NOT an engineer — never
writes logic, never makes design calls. Mechanical fixes on other
people's branches + verified-merge only.

Runs on cron `17 * * * *`. On first boot reads four handoff files +
the last 20 lines of cron-learnings.jsonl, waits for the scheduled
tick (no first-boot triage — known stale-state footgun).

## Files

org-templates/molecule-dev/triage-operator/
- system-prompt.md (48 lines) — role prompt loaded at boot. Standing
  rules, verification discipline, escalation paths.
- philosophy.md (135 lines) — 10 principles each tied to a real
  incident. Rule 2 ("tool succeeded ≠ work done") references the
  WorkOS refresh-token + missing-migration saga. Rule 3 (authority
  verification) references PR #370 CEO directive hold.
- playbook.md (234 lines) — step-by-step tick flow (Step 0 guards →
  1 list → 2 seven-gate → 3 docs sync → 4 issue pickup → 5 report).
  Expected 5–30 min wall-clock. When-not-to-triage.
- handoff-notes.md (146 lines) — point-in-time state for the NEXT
  operator arriving fresh. 15 PRs merged this session, in-flight
  items, design-call backlog with recommendations per issue.
- SKILL.md (152 lines) — installable skill spec. Invocation, inputs,
  outputs, required composed skills, edge cases, output format.

.claude/AGENT_HANDOFF.md (206 lines) — top-level handoff for any
Claude Code agent working this repo (not just the triage operator).
The 10 principles (one-liners), communication style the user
expects, currently-live state, open items, what NOT to do, break-
glass escalation conditions. Points at triage-operator/philosophy.md
for full incident context.

## Wiring

org.yaml gains a Triage Operator workspace block under PM with:
- tier: 3, model: opus
- 8 plugins (careful-bash, session-context, cron-learnings,
  code-review, cross-vendor-review, llm-judge, update-docs, hitl)
- Hourly cron at `:17` with the full Step 0–5 flow inline as prompt
- canvas position (1150, 250) — peer to Documentation Specialist

## Why this ships now

The 30-min manual triage cron was cancelled per CEO direction. The
role moves to another team. Without this handoff package they'd be
rediscovering the same incident-classes I shipped fixes for
(#318 fail-open, #327 cross-tenant decrypt, #351 tokenless grace,
WorkOS refresh-token saga, missing migration runner). The philosophy
file gives them the scar tissue in ~10 min of reading; the playbook
gives them the steps; the SKILL gives them an invocable entry point.

No code changes outside org.yaml. Existing TestPlugins_UnionWithDefaults
still passes (verified in platform test run).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/issue-319-encrypt-channel-tokens branch April 16, 2026 12:32
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
Wraps up a ~100-tick autonomous triage session by converting the prior
operator's institutional knowledge into standing, checked-in artifacts
so the next team picking up the hourly PR + issue cycle can drop in
without re-discovering everything from scratch.

## New role: Triage Operator

Peer to Dev Lead, Research Lead, Documentation Specialist under PM.
Owns the 7-gate PR verification + issue-pickup cycle across both
molecule-monorepo and molecule-controlplane. NOT an engineer — never
writes logic, never makes design calls. Mechanical fixes on other
people's branches + verified-merge only.

Runs on cron `17 * * * *`. On first boot reads four handoff files +
the last 20 lines of cron-learnings.jsonl, waits for the scheduled
tick (no first-boot triage — known stale-state footgun).

## Files

org-templates/molecule-dev/triage-operator/
- system-prompt.md (48 lines) — role prompt loaded at boot. Standing
  rules, verification discipline, escalation paths.
- philosophy.md (135 lines) — 10 principles each tied to a real
  incident. Rule 2 ("tool succeeded ≠ work done") references the
  WorkOS refresh-token + missing-migration saga. Rule 3 (authority
  verification) references PR #370 CEO directive hold.
- playbook.md (234 lines) — step-by-step tick flow (Step 0 guards →
  1 list → 2 seven-gate → 3 docs sync → 4 issue pickup → 5 report).
  Expected 5–30 min wall-clock. When-not-to-triage.
- handoff-notes.md (146 lines) — point-in-time state for the NEXT
  operator arriving fresh. 15 PRs merged this session, in-flight
  items, design-call backlog with recommendations per issue.
- SKILL.md (152 lines) — installable skill spec. Invocation, inputs,
  outputs, required composed skills, edge cases, output format.

.claude/AGENT_HANDOFF.md (206 lines) — top-level handoff for any
Claude Code agent working this repo (not just the triage operator).
The 10 principles (one-liners), communication style the user
expects, currently-live state, open items, what NOT to do, break-
glass escalation conditions. Points at triage-operator/philosophy.md
for full incident context.

## Wiring

org.yaml gains a Triage Operator workspace block under PM with:
- tier: 3, model: opus
- 8 plugins (careful-bash, session-context, cron-learnings,
  code-review, cross-vendor-review, llm-judge, update-docs, hitl)
- Hourly cron at `:17` with the full Step 0–5 flow inline as prompt
- canvas position (1150, 250) — peer to Documentation Specialist

## Why this ships now

The 30-min manual triage cron was cancelled per CEO direction. The
role moves to another team. Without this handoff package they'd be
rediscovering the same incident-classes I shipped fixes for
(#318 fail-open, #327 cross-tenant decrypt, #351 tokenless grace,
WorkOS refresh-token saga, missing migration runner). The philosophy
file gives them the scar tissue in ~10 min of reading; the playbook
gives them the steps; the SKILL gives them an invocable entry point.

No code changes outside org.yaml. Existing TestPlugins_UnionWithDefaults
still passes (verified in platform test run).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: workspace_channels.channel_config stores bot tokens in plaintext JSONB (inconsistent with workspace_secrets AES-256-GCM)

1 participant