Skip to content

fix: restrict billing channel picker to private channels only#661

Merged
bokelley merged 1 commit into
mainfrom
bokelley/addie-private-channels
Jan 7, 2026
Merged

fix: restrict billing channel picker to private channels only#661
bokelley merged 1 commit into
mainfrom
bokelley/addie-private-channels

Conversation

@bokelley

@bokelley bokelley commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restricts the billing channel picker to only show private channels for security (billing info shouldn't go to public channels)
  • Adds server-side validation to reject public channel IDs if someone calls the API directly
  • Shows a helpful empty state message when Addie isn't invited to any private channels

Test plan

  • Log in as admin and go to System Settings
  • Verify only private channels appear in the dropdown (Addie must be invited first)
  • Verify helpful message appears if no private channels are found
  • Try saving a public channel ID via API - should be rejected

🤖 Generated with Claude Code

For security, billing notifications should not go to public channels.
This change:
- Only shows private channels in the billing channel dropdown
- Adds server-side validation to reject public channel IDs
- Shows a helpful message when no private channels are found

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@bokelley bokelley merged commit 7325b0f into main Jan 7, 2026
6 checks passed
bokelley added a commit that referenced this pull request Apr 21, 2026
…oor rebaseline (#2639)

Adds the third cross-step assertion per expert review:

- After any denial signal (GOVERNANCE_DENIED, CAMPAIGN_SUSPENDED,
  PERMISSION_DENIED, POLICY_VIOLATION, TERMS_REJECTED, COMPLIANCE_UNSATISFIED,
  or check_governance status=denied) on a plan, no subsequent step may
  acquire a resource for that plan. Plan-scoped via plan_id; falls back
  to run-scoped for signals without plan linkage.
- Denial is sticky within a run — a later passing check_governance does
  not clear state. Requiring a fresh governance_context is correct
  semantics; this assertion encodes within-run monotonicity.
- Resource-acquired detection is task-allowlist (create_media_buy,
  update_media_buy, activate_signal, property/collection list CRUD,
  acquire_rights) + server-minted id fields. sync_* tasks deferred to
  follow-up (batch envelopes need schema-specific traversal).
- plan_id extraction reads response body first, then recorded request
  payload. Deliberately does NOT fall back to accumulated context —
  stale plan_id from earlier steps would bind denials to wrong plans.

Wired to governance-spend-authority (index + denied), governance-aware-seller,
governance-delivery-monitor. Silent on runs with no denial signal.

Tests: 41 passing including table-driven coverage of all 6 denial codes,
negative cases for excluded codes (GOVERNANCE_UNAVAILABLE, ACCOUNT_SUSPENDED,
CONFLICT, IDEMPOTENCY_CONFLICT), plan-scoped correctness, sticky state,
onStart reset, nested error.code shapes.

Rebases storyboard CI floors (legacy 35→27 clean, 279→271 passing;
framework 21→19 clean, 237→226 passing) to match @adcp/client 5.8.1
baseline. The drop is from upstream validation tightening (schema-driven
hooks #694, strict sync_creatives #661, refs_resolve cross-step #670,
branch_set grading #693, PRM enforcement #677), not this assertion —
the invariants: YAML additions are inert against the SDK-bundled
compliance cache until @adcp/client re-publishes its tarball. Follow-up
filed as adcp#2667 to diagnose the newly-failing storyboards.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 21, 2026
#2639) (#2663)

* spec(compliance): first cross-step assertions on universal/idempotency (#2639)

Wires two programmatic gates to the universal idempotency storyboard via
the new `invariants: [...]` YAML field (shipped in @adcp/client 5.8+):

- idempotency.conflict_no_payload_leak — IDEMPOTENCY_CONFLICT error
  bodies must contain only allowlisted envelope fields. Leaking cached
  payload fingerprints (budget, start_time, cached_payload, etc.) turns
  stolen idempotency keys into a read oracle. Converts the key_reuse_conflict
  phase's reviewer_check into runtime enforcement.

- context.no_secret_echo — no response on any step may echo bearer
  literals, the test-kit's declared api_key, or suspect field names
  (Authorization, api_key, bearer, x-api-key) at any depth.

Modules live in server/src/compliance/assertions/ and register via
@adcp/client/testing's registry at import time. Runners must load them
before calling runStoryboard (CLI --invariants flag, or direct import);
the runner throws at start on unresolved ids.

Bumps @adcp/client to ^5.8.1 for the registry re-exports.

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

* spec(compliance): wire assertion registrations into runners (#2639)

CI failed on the parent PR because adding `invariants: [...]` to
universal/idempotency.yaml made the SDK runner throw at start on unresolved
ids — no caller was loading the assertion modules. Wire them in at the
choke points every storyboard-running flow already touches:

- server/src/services/storyboards.ts — API / comply() flows go through
  this wrapper; importing the assertions as a side effect registers them
  before any downstream runStoryboard call.
- server/tests/manual/run-storyboards.ts — the CI entry point; explicit
  import keeps the registration visible to the runner script itself.
- server/tests/manual/run-one-storyboard.ts, storyboard-smoke.ts —
  standalone manual runners that don't touch the service wrapper.

Node's module cache dedupes the actual registerAssertion calls, so the
overlap is harmless.

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

* spec(compliance): governance.denial_blocks_mutation assertion + CI floor rebaseline (#2639)

Adds the third cross-step assertion per expert review:

- After any denial signal (GOVERNANCE_DENIED, CAMPAIGN_SUSPENDED,
  PERMISSION_DENIED, POLICY_VIOLATION, TERMS_REJECTED, COMPLIANCE_UNSATISFIED,
  or check_governance status=denied) on a plan, no subsequent step may
  acquire a resource for that plan. Plan-scoped via plan_id; falls back
  to run-scoped for signals without plan linkage.
- Denial is sticky within a run — a later passing check_governance does
  not clear state. Requiring a fresh governance_context is correct
  semantics; this assertion encodes within-run monotonicity.
- Resource-acquired detection is task-allowlist (create_media_buy,
  update_media_buy, activate_signal, property/collection list CRUD,
  acquire_rights) + server-minted id fields. sync_* tasks deferred to
  follow-up (batch envelopes need schema-specific traversal).
- plan_id extraction reads response body first, then recorded request
  payload. Deliberately does NOT fall back to accumulated context —
  stale plan_id from earlier steps would bind denials to wrong plans.

Wired to governance-spend-authority (index + denied), governance-aware-seller,
governance-delivery-monitor. Silent on runs with no denial signal.

Tests: 41 passing including table-driven coverage of all 6 denial codes,
negative cases for excluded codes (GOVERNANCE_UNAVAILABLE, ACCOUNT_SUSPENDED,
CONFLICT, IDEMPOTENCY_CONFLICT), plan-scoped correctness, sticky state,
onStart reset, nested error.code shapes.

Rebases storyboard CI floors (legacy 35→27 clean, 279→271 passing;
framework 21→19 clean, 237→226 passing) to match @adcp/client 5.8.1
baseline. The drop is from upstream validation tightening (schema-driven
hooks #694, strict sync_creatives #661, refs_resolve cross-step #670,
branch_set grading #693, PRM enforcement #677), not this assertion —
the invariants: YAML additions are inert against the SDK-bundled
compliance cache until @adcp/client re-publishes its tarball. Follow-up
filed as adcp#2667 to diagnose the newly-failing storyboards.

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

---------

Co-authored-by: Claude Opus 4.7 (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.

1 participant