Skip to content

test(security): backfill encryption tests for Telegram/WhatsApp/Slack-002 (#664)#876

Merged
vybe merged 2 commits into
devfrom
AndriiPasternak31/issue-664
May 23, 2026
Merged

test(security): backfill encryption tests for Telegram/WhatsApp/Slack-002 (#664)#876
vybe merged 2 commits into
devfrom
AndriiPasternak31/issue-664

Conversation

@AndriiPasternak31
Copy link
Copy Markdown
Contributor

Summary

Backfills the unit-test coverage that was missing for the AES-256-GCM
encryption already shipped on three channel bot-token columns:

Column Module Originating PR
telegram_bindings.bot_token_encrypted db/telegram_channels.py TELEGRAM-001
whatsapp_bindings.auth_token_encrypted db/whatsapp_channels.py WHATSAPP-001
slack_workspaces.bot_token db/slack_channels.py (SLACK-002) #453

These columns store an AES-256-GCM JSON envelope but had no dedicated
tests for the encryption invariant. This PR adds 31 unit tests following
the proven pattern from tests/unit/test_slack_token_encryption.py (#453).

Closes #664.

What each suite covers

  • Round-trip via the public ops API — write encrypts, read decrypts; callers never see the envelope.
  • Raw envelope inspection — on-disk value is {"algorithm": "AES-256-GCM", "nonce": …, "ciphertext": …}, never the original token.
  • Corrupt envelopeNone + ERROR log.
  • Wrong-key envelopeNone (AES-256-GCM auth-tag rejection, protects against key-substitution attacks).
  • Fresh nonce on update — token rotation produces a new envelope, not a re-used nonce.
  • Missing keyencrypt raises; decrypt returns None cleanly.

SLACK-002-specific

  • Plaintext fallback path at slack_channels.py:47-49 — legacy xoxb-* rows still readable; operator-facing WARNING log fires (caplog-asserted).
  • get_all_workspaces decrypts every row, not just the first.

WhatsApp-specific

  • account_sid plaintext invariant — pinned so future refactors don't accidentally encrypt the public Twilio identifier (which would silently break Twilio API calls).

Scope

Pure test backfill. No production code changes. No new dependencies.

Repo hygiene picked up along the way (CSO L-1 finding on this branch):

  • .gitignore: ignore the empty uv.lock that uv generates against this repo's pytest-only pyproject.toml.
  • docs/security-reports/cso-diff-2026-05-17.md: archived CSO --diff audit for this PR (verdict: CLEAR).

Pre-Landing Review

  • SQL: all queries use ? placeholders — clean.
  • Credentials: every fixture is unambiguously fake (xoxb-FAKE-TEST-FIXTURE-…, 123456:FAKE-…, alphabet-walk filler). secrets.token_hex(32) CREDENTIAL_ENCRYPTION_KEY rotated per test via monkeypatch autouse fixture.
  • Test isolation: per-test tmp_path SQLite + monkeypatch.setattr(module, "get_db_connection", …) — no shared state.
  • Conventions: matches the existing test pattern from fix(security): Encrypt Slack and Telegram bot tokens at rest (Invariant #13) #453.
  • sys.modules lint: passes (229 violations / 232 baseline, no new entries).
  • ruff check: passes.

CSO --diff verdict: CLEAR (no real secrets, no new auth surface, no new supply-chain surface).

Test plan

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

@vybe vybe left a comment

Choose a reason for hiding this comment

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

Substance is approved — 31 well-organized tests, follows the #453 pattern, fixtures are unambiguously fake (xoxb-FAKE-TEST-FIXTURE-… / alphabet-walk fillers), the WhatsApp suite correctly pins the account_sid plaintext invariant, CSO --diff CLEAR, pytest matrix green across all 6 seeds.

Two blockers before merge:

  • Rebase on current dev to fix the lint failure. Same root cause I flagged on #875: PR #881 landed on dev with the test_slot_per_slot_ttl.py baseline bump (0 → 6) needed after #871. This branch's merge-base is 98574f37, older than #881 (587599d4), so the lint job sees those 6 violations as new on this PR. Rebasing should drop the failure.

  • Untangle the 4 feature-flow doc edits from commit 744ab05d. That commit ("sync flows with recent merges") edits agent-monitoring.md, parallel-headless-execution.md, platform-settings.md, and public-agent-links.md — none related to #664. Against current dev they still diverge meaningfully (68/32/130/299 lines), so they're not just merge-noise. Please either: (a) confirm post-rebase they collapse to no-op, or (b) split that commit into a separate docs(feature-flows): sync … PR so this PR only carries the encryption-test backfill it advertises.

Re-approve immediately once CI is green and the scope is just the tests + .gitignore + CSO archive.

AndriiPasternak31 and others added 2 commits May 20, 2026 17:44
…-002 (#664)

Adds 31 unit tests covering the AES-256-GCM encryption that already shipped
on three channel bot-token columns:

- `telegram_bindings.bot_token_encrypted` (db/telegram_channels.py)
- `whatsapp_bindings.auth_token_encrypted` (db/whatsapp_channels.py)
- `slack_workspaces.bot_token` (db/slack_channels.py, SLACK-002)

Each suite covers: round-trip via the public ops API, raw DB envelope
inspection (AES-256-GCM JSON with nonce + ciphertext), corrupt-envelope
and wrong-key decryption failure, fresh-nonce on update, missing-key
behavior. Slack-002 additionally pins the plaintext-fallback path at
slack_channels.py:47-49 (legacy xoxb-* rows + operator WARNING log).
WhatsApp pins the `account_sid` plaintext invariant so future refactors
don't accidentally encrypt the public Twilio identifier.

Mirrors the test pattern established in #453
(tests/unit/test_slack_token_encryption.py) for SLACK-001 / db/slack.py.

Pure test backfill — no production code changes, no new dependencies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Adds `uv.lock` to .gitignore. The repo uses `pyproject.toml` only for
  pytest configuration (no `[project]` or `[tool.uv]` section). Running
  `uv` against this tree generates a 3-line empty lockfile with no
  `[[package]]` entries — checking it in adds churn with zero supply-chain
  signal. Flagged as L-1 in the #664 CSO diff audit.
- Adds the CSO diff audit report alongside the historical reports in
  docs/security-reports/. Verdict: CLEAR (test-only backfill, no new
  surface, fixtures unambiguously fake).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AndriiPasternak31 AndriiPasternak31 force-pushed the AndriiPasternak31/issue-664 branch from 744ab05 to 1ca38f2 Compare May 20, 2026 16:44
@AndriiPasternak31
Copy link
Copy Markdown
Contributor Author

Both blockers addressed:

1. Rebased on current origin/dev — picks up #881 baseline bump. Local lint now passes:

OK: 229 violation(s) in 66 file(s); baseline allows 235 — no new violations.
tests/unit/test_slot_per_slot_ttl.py: 6 → 0 (-6)  # under baseline, retired

2. Dropped the feature-flow doc commit (744ab05). You were right — those edits diverge meaningfully from current dev (origin/dev has since landed #873/#882/#884 revision-history rows on top). They aren't merge-noise and don't belong in this PR. I'll re-submit those flow updates as a separate docs(feature-flows): sync … PR.

Final scope (6 files):

  • tests/unit/test_slack_workspaces_encryption.py — 11 tests
  • tests/unit/test_telegram_token_encryption.py — 9 tests
  • tests/unit/test_whatsapp_token_encryption.py — 11 tests
  • tests/registry.json — entries for the 3 new files
  • .gitignore — ignore empty uv.lock
  • docs/security-reports/cso-diff-2026-05-17.md — archived CSO --diff audit (CLEAR)

Verification:

  • uv run pytest tests/unit/test_{slack_workspaces,telegram_token,whatsapp_token}_encryption.py31 passed
  • uv run python tests/lint_sys_modules.py → 0 new violations
  • mergeable now MERGEABLE (was CONFLICTING)

Ready for re-review.

Copy link
Copy Markdown
Contributor

@vybe vybe left a comment

Choose a reason for hiding this comment

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

Validated via /validate-pr — pure test backfill closing #664, 31 new tests, CI green, no production code changes.

@vybe vybe merged commit 3c3d930 into dev May 23, 2026
13 checks passed
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.

2 participants