Skip to content

feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection - #77

Closed
izzywdev wants to merge 3 commits into
feature/billing-paymentsfrom
billing-boundary
Closed

feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection#77
izzywdev wants to merge 3 commits into
feature/billing-paymentsfrom
billing-boundary

Conversation

@izzywdev

Copy link
Copy Markdown
Owner

Billing boundary hardening (per-service-DB direction)

billing-service must own ONLY the billing schema and never write the platform's public tables. This PR enforces that.

1. Least-privilege billing_svc Postgres role

  • backend/src/scripts/db-bootstrap.ts: idempotently creates/syncs a billing_svc LOGIN role (NOSUPERUSER/NOCREATEDB/NOCREATEROLE) and provisions a billing schema owned by it (CREATE SCHEMA ... AUTHORIZATION billing_svc) with USAGE/CREATE + DML + default privileges on that schema only. Explicitly REVOKE ALL ON SCHEMA public FROM billing_svc. Gated on BILLING_DB_PASSWORD so existing installs are a no-op.
  • Helm: db-bootstrap-job.yaml passes BILLING_DB_USER/BILLING_DB_PASSWORD; secret.yaml adds BILLING_DB_PASSWORD; values.yaml adds secret.billingDbPassword, sets billingService.dbUser: billing_svc, and the billing-service DATABASE_URL now connects as billing_svc (falls back to the shared role when the password is unset).

2. Removed cross-schema writes from billing-service

  • Deleted writePlanCache from HandlerContext (handlers/types.ts), its definition in index.ts, and every call (subscription-updated, invoice-paid, invoice-failed). invoice-failed now also emits billing.subscription.changed (status past_due) so the projection still gets the status change. Handlers continue to mirror the billing schema, sync Permit, and emit events.
  • Grep proof: no remaining public.users/public.organizations/writePlanCache/UPDATE public in services/billing-service/src (only doc comments).

3. Backend plan-state projection

  • New backend/src/services/billingProjection.ts consumes billing.subscription.changed (shared TypedConsumer + billingSubscriptionChangedSchemaV1) and updates billing_plan_tier/billing_plan_status on users/organizations, keyed by (entityType, entityId). Degrades to a no-op when Kafka is disabled. Wired into startServer + graceful shutdown.
  • Unit tests backend/tests/billing-projection.test.ts (mocked knex, no DB/broker).

4. Migration 010 collision resolved

  • Renumbered 010_add_billing_to_entities -> 011_add_billing_to_entities in both backend/src/migrations/ and backend/security/src/migrations/ (the identity track owns 010_create_api_tokens_table). Columns unchanged; the WRITER moved from billing-service to the backend projection. Comments updated.

Verification (Linux/CI for tsc+jest; locally for Helm)

  • helm template ... --set billingService.enabled=true → renders rc=0; DATABASE_URL = postgresql://billing_svc:$(BILLING_DB_PASSWORD)@...; with secret.billingDbPassword="" it falls back to fuzefront_user:$(DB_PASSWORD).
  • billing-service tsc/jest + backend tsc/jest to be confirmed green by CI (local Windows env has the os=linux npmrc pin that breaks native installs).

Base: feature/billing-payments (PR #66).

@izzywdev

Copy link
Copy Markdown
Owner Author

Superseded by #92: billing_svc least-privilege role + schema + bootstrap Job landed there.

@izzywdev izzywdev closed this Jun 23, 2026
@izzywdev

Copy link
Copy Markdown
Owner Author

Reopening — verification showed 3 files NOT on master: backend/src/migrations/011_add_billing_to_entities.ts, backend/src/services/billingProjection.ts, backend/tests/billing-projection.test.ts (the backend billing-projection work). Most of #77 landed via #92, but that part didn't. My 'superseded' close was on assumption, not verification — apologies.

@izzywdev izzywdev reopened this Jun 23, 2026
@izzywdev

izzywdev commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

@claude complete this branch and get it in.

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @izzywdev's task in 0s —— View job


I'll analyze this and get back to you.

@izzywdev

izzywdev commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Claude finished @izzywdev's task in 0s —— View job

I'll analyze this and get back to you.

@claude can you complete the PR to merge please? review its content against master and merge when ready

izzywdev commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

On it (via the Claude Code web session — the @claude GitHub bot ran on master HEAD, not this branch, which is the "0s" issue you saw).

Status:

The @claude 0s issue is also fixed — claude.yml now checks out the PR branch before running Claude, so future @claude comments on PRs will work correctly.


Generated by Claude Code

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @izzywdev's task in 4m 46s —— View job


PR #77 — billing boundary hardening: review & merge readiness

  • Read repo config + gather context
  • Review PR content (handlers, projection, migration, bootstrap, Helm)
  • Assess build/test verification status
  • Trigger CI (prior commits were all [skip ci])
  • Report readiness & merge status

Review verdict — the code is sound ✅

I read every changed file. The per-service-DB boundary is enforced correctly and the design is clean:

  • No residual cross-schema writes. grep over services/billing-service/src for writePlanCache / public.users / public.organizations / UPDATE public returns only doc comments — no actual writes. HandlerContext (handlers/types.ts) no longer exposes writePlanCache.
  • Handlers are consistent. subscription-updated, invoice-paid, and invoice-failed all mirror the billing schema → sync Permit → emit billing.subscription.changed. invoice-failed correctly emits status: 'past_due' so the projection still gets the transition (invoice-failed.ts:39).
  • Backend projection is correct & safe. applySubscriptionChanged keys on (entityType → users|organizations, entityId) and updates the two cache columns. startBillingProjection is a clean no-op when KAFKA_BROKERS is unset and never throws into the API. Wired into startServer (index.ts:500) and graceful shutdown (index.ts:378). All imports (createKafkaClient, TypedConsumer, TOPICS.BILLING_SUBSCRIPTION_CHANGED, billingSubscriptionChangedSchemaV1) resolve against @fuzefront/shared/kafka.
  • Migration renumber is clean. 011_add_billing_to_entities.ts exists in both backend/src/migrations and backend/security/src/migrations; no 010_* billing file remains. All column adds/drops are hasColumn-guarded (idempotent).
  • db-bootstrap.ts is idempotent and least-privilege. billing_svc is NOSUPERUSER/NOCREATEDB/NOCREATEROLE, owns the billing schema via AUTHORIZATION, gets DML + default privileges there only, and REVOKE ALL ON SCHEMA public. Gated on BILLING_DB_PASSWORD so existing installs are a no-op.

Minor observations (non-blocking)

  1. Two of the four new columns have no writer. The projection only writes billing_plan_tier + billing_plan_status; the migration also adds stripe_customer_id and trial_ends_at, and the billingSubscriptionChangedSchemaV1 event carries neither. Confirm these are intended to stay at defaults / be populated at customer-creation elsewhere. Fix this →
  2. Migration gap 009 → 011 on this branch (no 010) is intentional — the identity track owns 010_create_api_tokens_table. Knex tolerates the gap; just note ordering only lines up once that track merges.

⚠️ The one thing blocking "ready to merge": CI had never run

Every commit on this branch carried [skip ci], so tsc + jest have never executed on it (get_ci_status → 0 workflow runs). The PR description itself defers verification to CI. I can't run the build locally either — this sandbox blocks npm/network and git fetch (the checkout is shallow with orphan histories, so I also couldn't compute a true diff against master).

So I pushed an empty commit (4ebb7c5) without [skip ci] to kick off the pipeline. Once the backend and billing-service tsc/jest jobs go green, the code is merge-ready.

On performing the merge itself

I'm not able to merge the PR myself — merging/approving is disabled for the Claude GitHub app for security reasons (FAQ). Also worth flagging: this PR targets feature/billing-payments, not master — so "merge against master" would land here first, then flow up through PR #66.

Recommended path to get it in:

  1. Wait for the CI run I just triggered on 4ebb7c5 to pass.
  2. Merge PR feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection #77 into feature/billing-payments (or enable auto-merge) — a maintainer click, or I'm happy to re-verify once CI reports back.
    · branch billing-boundary

Prior commits on this branch carried [skip ci], so tsc+jest never ran.
This empty commit lets the pipeline produce the green signal the merge gate needs.

Co-authored-by: Israel Weinberg <izzywdev@users.noreply.github.com>
izzywdev added a commit that referenced this pull request Jul 9, 2026
…ith feature/billing-payments

Delivers all content from PR #77 (billing-boundary) with conflicts against
feature/billing-payments resolved:

- Least-privilege billing_svc Postgres role (db-bootstrap.ts + Helm)
- Removed cross-schema public.* writes from billing-service handlers
- Backend plan-state projection (billingProjection.ts + unit tests)
- Migration renumbered 010 → 011 to avoid collision with identity track
- claude.yml: checkout PR branch before running @claude (fixes 0s issue)

Closes #77
@izzywdev izzywdev closed this Jul 9, 2026
@izzywdev

izzywdev commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

@claude if you recommended to merge this PR, why was it closed?

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @izzywdev's task in 1m 14s —— View job


Why #77 was closed even though I recommended merging it

Short version: the recommendation and the closure don't actually conflict — the code got in through a different PR, and #77 was then closed as redundant. The close means "delivered elsewhere," not "rejected."

Here's the full chain from this PR's own timeline:

  1. 2026-06-23 — you first closed feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection #77 as "Superseded by feat(secrets): seal-secret.sh (credential-free) + fuzefront-secrets scaffold #92" (the billing_svc least-privilege role + schema + bootstrap Job landed there).

  2. 2026-06-23 — you reopened it, because verification showed 3 files from feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection #77 had not landed via feat(secrets): seal-secret.sh (credential-free) + fuzefront-secrets scaffold #92:

    • backend/src/migrations/011_add_billing_to_entities.ts
    • backend/src/services/billingProjection.ts
    • backend/tests/billing-projection.test.ts

    i.e. most of feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection #77 had landed, but the backend billing-projection slice hadn't.

  3. 2026-07-09 — you asked me to complete & merge. I reviewed every changed file, found the boundary hardening sound, and recommended merge conditional on CI going green (every prior commit carried [skip ci], so tsc+jest had never run on the branch). I also flagged two things I cannot do:

    • I can't merge/approve PRs — that's disabled for the Claude GitHub App for security reasons (FAQ).
    • This PR targets feature/billing-payments, not master.
  4. 2026-07-09you then set up the delivery path in your own comment: PR fix(billing-boundary): resolve merge conflicts with feature/billing-payments #206 (claude/pr-77-github-actions-l1qttvfeature/billing-payments) carried the full billing-boundary content plus the merge-conflict resolution and the claude.yml PR-branch-checkout fix. Your stated plan was:

    "Once it's green I'll merge fix(billing-boundary): resolve merge conflicts with feature/billing-payments #206feature/billing-payments, which delivers all the content from this PR. This PR (feat(billing): per-service-DB boundary hardening — billing_svc role + backend projection #77) can then be closed as superseded."

So #77 was closed exactly per that plan: its content was merged into feature/billing-payments via #206 (which also cleaned up the billing-boundary ↔ base merge conflicts that #77 itself couldn't carry). Closing #77 avoids merging the same changes twice.

If you want to confirm the content actually landed

I couldn't run gh/git fetch in this sandbox (network + non-interactive-approval blocked), so I'm reporting from the recorded PR history rather than a live check. To verify the backend projection slice is on the target branch:

git fetch origin feature/billing-payments
git ls-tree origin/feature/billing-payments -- \
  backend/src/services/billingProjection.ts \
  backend/src/migrations/011_add_billing_to_entities.ts \
  backend/tests/billing-projection.test.ts
gh pr view 206 --json state,merged,mergedAt,baseRefName

If all three files are present on feature/billing-payments and #206 shows merged: true, then #77 is correctly closed-as-superseded and nothing further is needed. If any file is missing, the projection slice slipped through the cracks again (as it did once with #92) and should be re-landed — tell me and I'll open a fresh PR from this branch with just those files.
· branch claude/pr-77-20260709-1033

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