Skip to content

fix: restore dead prod verification gates; wire root org, root admin, and Permit ReBAC - #453

Merged
izzywdev merged 4 commits into
masterfrom
claude/prod-verification-and-root-org
Jul 29, 2026
Merged

fix: restore dead prod verification gates; wire root org, root admin, and Permit ReBAC#453
izzywdev merged 4 commits into
masterfrom
claude/prod-verification-and-root-org

Conversation

@izzywdev

Copy link
Copy Markdown
Owner

📋 Description

Two clusters of defect, both of the same shape: machinery that exists, looks configured, and does nothing.

A. Production had no automated post-deploy verification for ~4 weeks

  • prod-smoke.yml triggered on push to values-prod.yaml gated if: startsWith(head_commit.message, 'release:'). Those conditions are mutually exclusive: release.yml writes its tag bump as release: fuzefront images <sha> [skip ci] — it must carry that token, or the bump push re-triggers release forever — and GitHub does not start push-triggered workflows for commits carrying it. Last real execution 2026-06-30; all 25 runs since are skipped; no run exists for any automated release commit.
  • prod-post-deploy.yml triggered on workflow_run of release.yml. It is registered, state: active, name matcher byte-exact, on master since 2026-07-27, and release.yml has succeeded many times since — yet zero runs, ever. So the post-prod Playwright E2E never ran automatically either.

Dispatching post-prod-e2e.yml by hand against production returned 5 passed, 3 failed — see Questions for Reviewers. Those failures are pre-existing and were invisible precisely because both gates were dead.

B. The platform root org and the Permit ReBAC hierarchy were unwired

  • No stable root identity. ensureRootPortal() resolved the root as the oldest organizations row of type='platform', creating one on the fly; its own doc comment conceded the codebase "has no pre-existing single root org concept". Rebuild the DB — as the 2026-07-24 Longhorn incident did — and a different row becomes root, orphaning every Permit tenant, tuple and portal row keyed to the old id.
  • Ownership fell to a service principal. Owner was "first user with admin in roles, else first user". In production that resolves to platform-registrar: migration 014 gives it roles ['admin','user'] and creates it before any human signs up. It has no password_hash and can never complete an interactive login — so the root org was owned by a principal no human can act as.
  • ReBAC declared, never wired. permit/schema.ts defines Organization.relations.parent and an org-admin role derived parent→child. createOrganizationResourceInstance / setOrganizationParent / assignOrgAdminRebac had zero callers anywhere in the repo. No resource instance, no parent tuple, no root grant were ever created — the derivation had nothing to derive from and failed closed, silently. parent_id was validated, permission-checked and stored, then handed to Permit only as a tenant attribute.
  • Permit schema never synced automatically — reachable only via npm run permit:schema and tests.

🔄 Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🧪 Test addition or improvement

🧪 Testing

  • Unit tests
  • Integration tests
  • E2E tests
  • Manual testing

Test Configuration:

  • Node.js version: 20 / 22 / 24 (CI matrix)
  • OS: Linux
  • Browser (if applicable): n/a

Test Instructions

cd backend
npx tsc --noEmit -p tsconfig.json
npm test -- provisioning.test.ts root-org-admin.test.ts

What I verified locally: tsc --noEmit is clean across all changed files (the only remaining errors are pre-existing @fuzefront/custom-hostname-client module-resolution failures, present on master). All four workflow files parse as valid YAML.

What I could NOT verify locally, and why: the backend suite needs Postgres and there is no Docker daemon in this environment, so provisioning.test.ts and the new root-org-admin.test.ts have not been executed — CI is their first run. The production endpoints are also unreachable from here (the egress proxy denies app.fuzefront.com: connect_rejected, gateway answered 403), which is why the post-prod E2E had to be dispatched into Actions instead.

🔧 Implementation Details

Changes Made

  • Backend Changes:
    • migrations/015_seed_root_platform_organization.ts (new) — seeds the root platform org under a fixed id, plus its owner membership. A migration, not a seed, for the reason migration 014 documents in its own header: runSeeds() only runs when NODE_ENV !== 'production'. It adopts a pre-existing platform org rather than creating a second one, so environments that already ran the old ensureRootPortal() don't end up with two platform orgs and "oldest wins" deciding which is real.
    • repositories/portalRepository.ts — resolve the root org by fixed id, falling back to oldest-platform for pre-015 databases; pin the created row to that id.
    • services/organizationProvisioning.ts — new permit_org_instance and permit_org_parent steps. A missing explicit parent fails the step (a real inconsistency the caller asked for); a missing implied root parent is skipped with a warning, so org creation can never wedge in failed on a DB without a root org. Authorization still fails closed either way.
    • services/rootOrgAdmin.ts (new) — grants root org-admin to real administrators on boot, explicitly excluding platform-registrar. Granting that token-only principal tree-wide authority would hand every holder of a sealed registration token administrative control of every tenant — a privilege escalation, not a convenience. One failing grant does not abort the rest.
    • permit/sync-permit-schema.ts — extract syncPermitSchemaFromRegistry() so boot and the CLI share one definition. Syncing the base schema alone would omit product policies, and a role absent from the synced schema just denies, silently.
    • index.ts — sync the Permit schema and ensure root admins at boot. Both non-fatal: Permit being unreachable must not stop the platform booting.
  • CI Changes:
    • release.yml ends with an explicit gh workflow run prod-post-deploy.yml. workflow_dispatch is exempt from the GITHUB_TOKEN recursion guard (the property auto-merge.yml already relies on) and immune to the suppression token. Non-blocking, but a failed dispatch emits a warning naming the release UNVERIFIED.
    • prod-post-deploy.yml — dispatch-triggered; chains health → E2E; no longer cancel-in-progress (a cancelled verification leaves a deploy unverified, the exact failure it exists to prevent).
    • prod-smoke.yml — reusable workflow_call + workflow_dispatch instead of a push trigger that could never fire.
    • Each file records why the previous trigger was dead, so this is not quietly reintroduced.
  • Frontend Changes: none.
  • SDK Changes: none.

Code Quality

  • Code follows the project's coding standards
  • Self-review of code completed
  • Code is commented, particularly in hard-to-understand areas
  • No console.log or debugging statements left in code

Documentation

  • Documentation has been updated (if applicable)
  • API documentation updated (if applicable)
  • README updated (if applicable)
  • Migration guide provided (for breaking changes)

🚨 Breaking Changes

None to any API or package surface. Migration 015 is additive and idempotent, and adopts rather than replaces an existing platform org. Its down() is deliberately irreversible — deleting the root org would orphan the root portal and every child's parent tuple.

📋 Checklist

Pre-submission

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes — not run locally, see Testing

Code Quality

  • Code follows conventional commit format
  • TypeScript strict mode passes (no new errors vs. baseline)
  • ESLint passes without errors — not run on backend locally
  • No security vulnerabilities introduced
  • Performance impact considered and documented

Security Checklist (if applicable)

  • No sensitive data exposed
  • Input validation implemented — unchanged; server-side validation remains authoritative
  • Authentication/authorization properly handled — this PR narrows effective authority: platform-registrar is explicitly excluded from the root org-admin grant. Wiring the hierarchy grants no permission that the schema did not already intend.
  • No SQL injection vulnerabilities — parameterized knex.raw throughout
  • XSS prevention measures in place — n/a

🔗 Related Issues and PRs

Follows #446 (create-organization UI fix), which is what surfaced the root-org and Permit questions.

🎯 Reviewers

  • Subject matter expert (if applicable)

📝 Additional Notes

Deployment Notes

  • Requires database migration — 015, additive and idempotent, runs automatically on backend start
  • Requires environment variable changes
  • Requires dependency updates
  • Requires configuration changes

Boot-order note: the Permit schema sync and root-admin grant run on every start and are non-fatal. On a Permit outage the platform still boots; authorization continues to fail closed.

Future Work

  • The post-prod E2E does not cover organization creation, so it would not have caught fix(frontend): make "create organization" actually reach the API #446. Worth adding.
  • ensureRootPortal() still leaves root ownership on platform-registrar until a human admin exists; promoting ownership to a real administrator when one appears is a natural follow-up.
  • The root-admin grant keys off roles LIKE '%admin%', a substring match on a JSON column. It works, but a proper role predicate would be sturdier.

Questions for Reviewers

  • The dispatched post-prod E2E failed against live production — 5 passed, 3 failed (run 30449148959). These are pre-existing and unrelated to this diff, but you should see them:

    • "Sign in with Google" hands off to Authentik (OIDC flow starts)
    • authenticated dashboard + Module-Federation apps load
    • Authentik login surface is configured — fails with enroll_url missing — enrollment flow not linked on the identification stage (sign-up dead)

    The two prior manual dispatches (2026-07-21, 2026-07-23) also failed, so this has been red for a while with nothing reporting it. Note the authenticated-dashboard case may partly be an artifact: the spec falls back to seeded admin credentials when POST_PROD_EMAIL/POST_PROD_PASSWORD are unset, and the seeded admin does not exist in production (seeds never run there) — so that test may have no valid login available rather than a broken dashboard. Worth setting those secrets before reading too much into it.

  • Root-org ownership: I seeded it owned by platform-registrar so the migration has no dependency on a human existing. If you'd rather it be owned by a named human from the start, that needs a decision on who — say the word and I'll change it.

  • I did not file the Jira tickets we discussed; still waiting on which project/scope you want.


📊 Performance Impact

Bundle Size Impact:

  • No significant impact

Runtime Performance:

  • No significant impact — two extra Permit calls per org provisioning (idempotent, skipped on re-runs), and two boot-time calls.

🌐 Browser Compatibility

Not applicable — backend and CI only.

🔄 Backwards Compatibility

  • Fully backwards compatible

Generated by Claude Code

claude added 2 commits July 29, 2026 16:22
Production has had NO automated post-deploy verification for ~4 weeks.
Two independent gates were silently dead:

prod-smoke.yml triggered on push to values-prod.yaml gated
`if: startsWith(head_commit.message, 'release:')`. Those conditions are
mutually exclusive: release.yml writes its tag bump as "release: ...
[skip ci]" (it must, or the bump push re-triggers release forever), and
GitHub does not start push-triggered workflows for commits carrying that
token. Last real execution was 2026-06-30; all 25 runs since are
`skipped`, and no run exists for any automated release commit.

prod-post-deploy.yml triggered on `workflow_run` of release.yml. It is
registered, `state: active`, its name matcher is byte-exact, it has been
on master since 2026-07-27, and release.yml has succeeded many times
since — yet it has ZERO runs, ever. So the post-prod Playwright E2E
never ran automatically either.

- release.yml: end the release with an explicit `gh workflow run
  prod-post-deploy.yml`. workflow_dispatch is exempt from the
  GITHUB_TOKEN recursion guard (the property auto-merge.yml already
  relies on) and immune to the suppression token. Non-blocking, but a
  failed dispatch emits a warning naming the release as UNVERIFIED.
- prod-post-deploy.yml: dispatch-triggered; chains health then E2E, and
  no longer cancels in progress (a cancelled verification leaves a
  deploy unverified, the exact failure it exists to prevent).
- prod-smoke.yml: becomes a reusable workflow_call + workflow_dispatch
  instead of a push trigger that could never fire.

Each file records why the previous trigger was dead, so this is not
quietly reintroduced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012fmSD2gTMPWvpV9ZV4nHZU
… ReBAC

The Permit ReBAC hierarchy was declared but never wired, and the root
organization was a convention rather than an identity.

Root organization had no stable identity. `ensureRootPortal()` resolved it
as "the oldest organizations row of type='platform'", creating one on the
fly — its own doc comment conceded the codebase "has no pre-existing single
root org concept". Rebuild the DB (as the 2026-07-24 storage incident did)
and a different row becomes root, orphaning every Permit tenant, tuple and
portal row keyed to the old id.

Ownership fell to a service principal. The owner was "first user with admin
in roles, else first user". In production that resolves to
platform-registrar: migration 014 gives it roles ['admin','user'] and it is
created before any human signs up. It has no password_hash and can never
complete an interactive login, so the platform root org was owned by a
principal no human can act as.

ReBAC was declared but had zero callers. permit/schema.ts defines
Organization.relations.parent and an `org-admin` role derived parent->child.
createOrganizationResourceInstance / setOrganizationParent /
assignOrgAdminRebac had no callers anywhere in the repo, so no resource
instance, no parent tuple, and no root grant were ever created. The
derivation had nothing to derive from and failed closed, silently.
parent_id was validated, permission-checked and stored, then handed to
Permit only as a tenant attribute.

- migration 015: seed the root platform org under a FIXED id, adopting a
  pre-existing platform org rather than creating a second one. A migration,
  not a seed, for the reason 014 documents: seeds never run in production.
- portalRepository: resolve the root org by fixed id, falling back to
  oldest-platform for databases that predate 015.
- organizationProvisioning: add permit_org_instance and permit_org_parent
  steps. A missing EXPLICIT parent fails the step; a missing implied root
  parent is skipped with a warning so org creation can never wedge.
- services/rootOrgAdmin: grant root org-admin to real administrators on
  boot, explicitly EXCLUDING platform-registrar — granting it tree-wide
  authority would hand every holder of a sealed registration token
  administrative control of every tenant.
- index.ts: sync the Permit schema at boot (previously reachable only via
  `npm run permit:schema`), merged with product policies via a shared
  helper so the base schema never clobbers them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012fmSD2gTMPWvpV9ZV4nHZU
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

@github-actions
github-actions Bot enabled auto-merge (squash) July 29, 2026 16:30
Comment thread backend/src/services/rootOrgAdmin.ts Fixed
`organization_provisioning.step` is a native Postgres enum created by
migration 009 with exactly four members. Adding permit_org_instance and
permit_org_parent to PROVISIONING_STEPS without extending the enum made
ensureStepRows() fail with "invalid input value for enum
provisioning_step_enum", which aborts the whole multi-row insert — so NO
step rows were created and organization provisioning stopped working
entirely, not just for the new steps. Caught by Backend tests (Node 24.x).

Also address the Semgrep unsafe-formatstring finding: use constant format
strings with arguments instead of interpolating ids into the format
string, where a value containing format specifiers could forge the log
line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012fmSD2gTMPWvpV9ZV4nHZU
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

001_initial_users.ts did a bare `organizations().del()`, which deleted the
root platform org that migration 015 had just created. The root org
therefore existed only until the first seed run — after which
ensureRootOrgAdmins() had nothing to grant on (it returns [] with no root
org) and every new org skipped its `parent` link to the root, so the
ReBAC derivation still resolved to nothing. That is what failed 2 suites
/ 6 tests on the previous commit.

The file already exempts PLATFORM_REGISTRAR_ID from the users delete for
exactly this reason, and documents that the row has taken production down
three times. This applies the same exemption to the root organization,
its membership, and its provisioning rows — plus its owner, since
organizations.owner_id CASCADEs and deleting the owner would undo the
exemption.

Production never runs seeds (initializeDatabase gates on NODE_ENV), so
this bites dev and CI only — precisely where it is hardest to notice and
easiest to mistake for "the feature doesn't work".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012fmSD2gTMPWvpV9ZV4nHZU
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

@izzywdev
izzywdev merged commit 9cab21f into master Jul 29, 2026
53 checks passed
@izzywdev
izzywdev deleted the claude/prod-verification-and-root-org branch July 29, 2026 21:53
izzywdev pushed a commit that referenced this pull request Jul 29, 2026
PR #453 landed root-org seeding, root-admin granting, and the Permit ReBAC
wiring while this branch was parked on review — more completely than this
branch did. It adds migration 015_seed_root_platform_organization (root at
the fixed id ...0010, slug `fuzefront`, ADOPTING any pre-existing
type='platform' row instead of creating a second), a rootOrgAdmin service
that grants root org-admin on boot, callers for the previously-unreferenced
createOrganizationResourceInstance/setOrganizationParent/assignOrgAdminRebac,
and the same "stop deleting the root org in the dev seed" fix.

Keeping this branch's 015_seed_fuzeone_root_org would have been actively
harmful, not merely redundant. Knex orders migrations by filename, so
`015_seed_fuzeone_root_org` sorts BEFORE `015_seed_root_platform_organization`.
On any fresh database both would run: ours would create a platform org at
...0002, then theirs would find a platform org whose id != ...0010, log
"adopting existing ... NOT repointing", and return without ever creating
...0010. portalRepository has a fallback lookup, but rootOrgAdmin resolves
ROOT_ORG_ID directly — so it would find no root org and grant no root admin,
silently re-breaking the exact ReBAC derivation #453 fixed.

- Delete backend/src/migrations/015_seed_fuzeone_root_org.ts.
- Take master's backend/src/seeds/001_initial_users.ts wholesale: it also
  exempts the root org from the dev reset, and additionally keeps the root
  org's OWNER, without which the users delete cascades the org away via
  organizations.owner_id ON DELETE CASCADE and undoes its own exemption.

What remains on this branch is only what #453 did not cover: the production
invite drive spec. Its org selector now matches the root org by its
`platform` TYPE rather than by name, because #453's adopt-don't-replace
behaviour means the root org's name is environment-dependent (seeded as
`FuzeFront`, but an adopted row keeps whatever it was called) while "the
single platform-type org" is stable. INVITE_ORG_NAME still overrides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMV85A962htFky7NYFJExX
izzywdev pushed a commit that referenced this pull request Jul 29, 2026
…r merge

Master's #453 landed `015_seed_root_platform_organization.ts` and
`016_provisioning_steps_rebac.ts` while this branch already carried its own
`015_app_scope_levels_and_installations.ts`. Two files sharing ordinal 015 is
not a conflict git can see -- the merge was clean -- but knex sorts migrations
lexically by filename, so the relative order of two 015s is decided by the
rest of the string, not by intent. Renumbered to 017 so the ordering is
explicit and the app-scopes work runs after the root-org/ReBAC provisioning it
now sits behind.

Safe to rename rather than add a no-op shim: this branch has never been
deployed, so no `knex_migrations` row anywhere records the old filename.

Also updated the "migration 015" comment references that pointed at this
file (backend/src/routes/app-installations.ts, backend/src/routes/apps.ts,
backend/tests/app-installations.test.ts, frontend/src/services/api.ts).
Master's own references to its 015 are untouched.

Verified on a fresh Postgres 16 with every migration replayed in the new
order: knex_migrations ends 014_seed_platform_registrar_user,
015_seed_root_platform_organization, 016_provisioning_steps_rebac,
017_app_scope_levels_and_installations; `apps.scope_level` present; all
app_installations indexes and the shape CHECK present. Backend suite
91/91 passing (--runInBand; parallel runs contend on knex's migration lock).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un
izzywdev added a commit that referenced this pull request Jul 29, 2026
Adds a production drive spec for the organization invite flow: sign in,
select the root org, Members tab, invite as admin, assert the pending
invitation carries that role — with the console-clean runtime gate.

The org is matched by its `platform` TYPE rather than by name, because
migration 015_seed_root_platform_organization adopts a pre-existing
platform row instead of creating a second one, so the root org's name is
environment-dependent while "the single platform-type org" is not.
INVITE_ORG_NAME overrides.

409 from POST /organizations/:id/invitations is treated as success so a
re-run converges on the same verified end state; the assertion is on what
the org shows, not on the click that produced it.

The spec mutates its target (real invitation + real invite email event),
so it is opt-in via AUTHN_TEST_EMAIL/AUTHN_TEST_PASSWORD and self-skips
otherwise. It has NOT yet been run green against a live target.

Scope note: this branch originally also added a FuzeOne root-org migration
and a dev-seed fix. #453 landed that work more completely while this was in
review, and keeping this branch's 015_seed_fuzeone_root_org would have been
harmful — it sorts before #453's migration by filename, so it would create a
platform org at ...0002, make #453's adopt-guard early-return without ever
creating ...0010, and leave rootOrgAdmin with no root org to grant on.
izzywdev added a commit that referenced this pull request Aug 2, 2026
…count, notification service (#449)

* feat(shell): app install scopes, consolidated identity menu, multi-account, notification service

Four changes to the shell's identity surface plus the inbox behind the bell.
Plan of record: docs/planning/app-scopes-user-menu-notifications.md.
Approval frames: design/frames/app-scopes-user-menu/ (5 frames, stamped).

App install scopes
------------------
`apps` already answered who OWNS an app (organization_id) and who may SEE it
(visibility). Neither answered who it may be INSTALLED for, and there was no
installation record at all.

- migration 015: apps.scope_level (personal|organization|both, default both)
  and app_installations. Shape is enforced in the DATABASE — a CHECK constraint
  pins which anchor columns each (scope, install_mode) combination carries, and
  three partial unique indexes make install idempotent per target. They are
  scoped to status='active' so uninstall can be a soft revoke and a reinstall
  does not collide with a stale row.
- The column is scope_level, never scope: apps.scope already means the
  Module-Federation remote container name.
- POST /api/apps/:id/install, DELETE .../install/:installationId,
  GET .../installations, GET /api/apps/installed. Fail-closed at each step:
  an app the caller cannot see is 404 (never 403 — no id probing), a scope the
  app's level forbids is 422, an org install needs active membership, and
  mode=everyone additionally needs owner/admin. One member must not be able to
  push an app into every colleague's launcher.
- InstallAppDialog asks only what the app and the caller's role leave open.

Consolidated identity menu
--------------------------
The organization switcher and the language selector move out of the top bar and
into the avatar menu, alongside a new account switcher. Six top-bar controls did
not fit a phone, and three of them answered the same question: who am I.

Sections fail INDEPENDENTLY — a failed org fetch renders in place with a retry
while accounts, language and sign-out stay usable, rather than taking the whole
identity surface down and stranding the user with no way to sign out.

Multi-account (MAX_PARALLEL_ACCOUNTS = 5)
-----------------------------------------
lib/accounts.ts is the vault. Three rules:

1. Namespacing — every per-account value lives under ff.acct.<id>.<key>. The
   roster holds no credentials. The pre-existing bare authToken/sessionId/user/
   ff.activeOrganizationId keys are migrated in at boot and then deleted, so a
   stale global can never shadow a namespaced value.
2. One resolver — getActiveAuthToken() is the only way to obtain a token. The
   axios interceptor, the federated-app loader, the chat widget, the flags
   client and the account-security page all read through it, so a token can
   never be read for a non-active account.
3. Per-tab active account — sessionStorage first, localStorage default second.
   That is what makes the accounts parallel: account A in tab 1 and account B in
   tab 2, each pinned to its own identity.

Switching is a full document teardown, not a state update: React state, the
app-registry cache, the flag cache, mounted remotes and any open stream die with
the document. A soft in-place switch would leave all of those holding account
A's data while account B's token is on the wire.

At the cap a sixth sign-in is REFUSED, not absorbed by evicting an existing
account. Signing out one account revokes only that session — using that
account's own token, not the shared axios client's.

Stated limit: same-origin isolation is a storage and lifecycle boundary, not a
browser security boundary. This bounds accidental leakage; it does not claim to
bound a compromised origin.

Notification service
--------------------
services/notification-service — Express + Postgres, reached same-origin through
a new host-backend proxy at /api/v1/notifications.

- One row per RECIPIENT, not per event: fan-out happens at write time, so the
  read path is a single indexed scan and the badge is a partial-index count.
- read / seen / archived are separate columns. Opening the panel marks seen —
  the badge clears because the user looked — and does not silently mark
  everything read.
- A per-user dedupe_key, unique among live rows, means a retried producer cannot
  double-post. Suffixed per recipient so one producer key deduplicates each
  person's copy independently.
- Every user-facing route derives user_id from the verified JWT. No route
  accepts a user id, so there is nothing to tamper into another mailbox.
- /internal/publish is the one privileged route, gated on a shared service
  token, and it FAILS CLOSED (503) when that token is unset. The host-backend
  proxy refuses to forward /internal/* at all, so it is never one header away
  from the browser.
- SSE for live delivery, with a per-user stream cap and a heartbeat. The hub is
  in-process: with replicas > 1 a live push only reaches clients on the same
  pod, which degrades to a delayed badge that the client's reconnect + refetch
  corrects. Documented in values.yaml next to `replicas`.
- The bell degrades quiet — service down means no badge, never a broken shell.

Verification
------------
- backend: 26 app-installation tests + 10 proxy tests pass against real
  Postgres, exercising the CHECK constraint and the partial unique indexes;
  pre-existing apps suite still 37/37.
- notification-service: 35 tests (routes, SSE hub, auth middleware).
- frontend: 111 tests including 26 new vault tests; vite build clean.
- backend + frontend type-check and lint clean; gate-ds-conformance clean on
  changed lines; frames stamped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* fix(security): address CodeQL + Semgrep findings on the new routes

CodeQL flagged 7 alerts on this PR (5 high, 2 medium). Triaged individually
rather than blanket-suppressed.

Missing rate limiting — app-installations.ts (4 alerts, REAL)
------------------------------------------------------------
Every route there performs an authorization decision backed by DB lookups (app
visibility + org membership), so an unbounded caller amplifies cheap requests
into repeated database work. Worse, because a not-entitled app or org answers
404 by design, an unbounded caller is exactly the shape of an id-enumeration
probe — the 404 that protects against disclosure is only as good as the rate at
which it can be sampled.

Adds read (60/min) and write (30/min) limiters following the repo's existing
express-rate-limit convention (routes/flags.ts, routes/portal.ts).

Both ceilings are env-overridable, and the limiter is ALWAYS mounted — never
skipped by NODE_ENV. A security control that switches itself off outside
production is not a control. The suite raises the ceiling instead (it makes far
more calls per minute than any real user, all from one loopback IP and so one
bucket), and a new test asserts the limiter actually returns 429 once exceeded.
Silencing the alert without proving the fix works would have been the easy
version of this.

Log injection + externally-controlled format string — notifications.ts (2 alerts, REAL)
--------------------------------------------------------------------------------------
The proxy logged `req.method` and `req.url` interpolated into a template
literal. Both are attacker-controlled, which allowed two things: a CR/LF in the
URL forges an extra, fabricated log entry — the one place an attacker gets to
write the record of their own activity — and a %s/%d is consumed as a console
format specifier, shifting every later argument.

Fixed at both call sites: the format string is now a constant literal with the
values passed as arguments, and each value goes through safeForLog(), which
strips control characters, escapes format specifiers, and bounds the length. A
truncated, flattened URL still identifies the failing route.

Hardcoded JWT secret in tests — Semgrep (6 findings, ACCEPTED)
-------------------------------------------------------------
These are test-only signing keys: they exist so the suites can mint tokens the
middleware under test will verify, they appear nowhere else, and they
authenticate nothing outside the test process. Production reads JWT_SECRET and
NOTIFICATION_INTERNAL_TOKEN from the environment, and both surfaces fail CLOSED
when unset — which the suite itself asserts.

Suppressed with `// nosemgrep: <rule>` at each site plus one justification block
per file, matching the repo's existing pattern (backend/src/routes/auth.ts,
tests/e2e/billing-invoices). Explicit and reviewable rather than a .semgrepignore
entry that would also hide future real findings in those files.

Also merges origin/master (portal epic, PR #424) — no conflicts; migration 012
lands below this branch's 015.

Verification: 37 backend tests (installations + proxy) against real Postgres,
35 notification-service tests, backend type-check + lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* fix(e2e): read the session through the account vault, not a bare authToken key

The Playwright sign-in and OIDC-plumbing suites went red on this branch. Real
breakage caused by this PR, not flaky infra: nine spec files assert on
`localStorage.getItem('authToken')`, and the account vault moved every
per-account value under `ff.acct.<accountId>.<key>`.

The specs were right to check "is there a usable session" — they were just
reading a key that no longer exists. A bare `authToken` surviving would now MEAN
the isolation model had been bypassed, so the fix is to resolve the session the
way the app does, not to re-add the key.

Adds tests/support/account-vault.ts with readActiveAuthToken() /
hasActiveSession() / PROVISIONAL_ACCOUNT_ID, and points every call site at it:
auth-simple, auth, oidc-plumbing, google-signin, google-oauth-e2e,
prod-full-auth-flow, clock-load, pages/login-page, plus the seed path in
federated-apps-register-activate (which now writes into the provisional
namespace, where a real first sign-in parks its token).

The helper deliberately RE-IMPLEMENTS the vault's active-account resolution:
page.evaluate() ships the function source to the browser, so it cannot import
anything and must be self-contained.

That duplication is a drift risk, and the seam had NO guard at all —
frontend/tsconfig.json includes only `src`, so the spec files are not even
type-checked (verified by probe, not assumed). So this also adds
src/__tests__/e2e-account-vault-helper.test.ts, which cross-checks the helper
against the real vault across every state that matters — signed out, one
account, several accounts, per-tab pin, first sign-in in the provisional
namespace, add-account mode, last-account sign-out, stale pin, corrupt roster —
and asserts the helpers stay serializable. When the vault changes, that fails in
milliseconds with an obvious message instead of turning a 7-minute e2e run red
in a way that looks like sign-in broke.

Verification: 122 frontend unit tests (17 files) pass; every touched spec
compiles under `playwright test --list` (both the default and prod configs),
which is what actually type-checks these files; frontend lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* fix(migrations): renumber app-scopes migration 015 -> 017 after master merge

Master's #453 landed `015_seed_root_platform_organization.ts` and
`016_provisioning_steps_rebac.ts` while this branch already carried its own
`015_app_scope_levels_and_installations.ts`. Two files sharing ordinal 015 is
not a conflict git can see -- the merge was clean -- but knex sorts migrations
lexically by filename, so the relative order of two 015s is decided by the
rest of the string, not by intent. Renumbered to 017 so the ordering is
explicit and the app-scopes work runs after the root-org/ReBAC provisioning it
now sits behind.

Safe to rename rather than add a no-op shim: this branch has never been
deployed, so no `knex_migrations` row anywhere records the old filename.

Also updated the "migration 015" comment references that pointed at this
file (backend/src/routes/app-installations.ts, backend/src/routes/apps.ts,
backend/tests/app-installations.test.ts, frontend/src/services/api.ts).
Master's own references to its 015 are untouched.

Verified on a fresh Postgres 16 with every migration replayed in the new
order: knex_migrations ends 014_seed_platform_registrar_user,
015_seed_root_platform_organization, 016_provisioning_steps_rebac,
017_app_scope_levels_and_installations; `apps.scope_level` present; all
app_installations indexes and the shape CHECK present. Backend suite
91/91 passing (--runInBand; parallel runs contend on knex's migration lock).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* fix: unbreak chat-service tsc — pin notification-service to the Node 24 typings floor

"Chat service (unit)" was the only red check of 59 on this PR. It failed in
`services/chat-service/src/rag/index-docs.ts` with five errors rooted in one:

    Type 'Dirent<string>[]' is not assignable to type 'Dirent<Buffer>[]'

Two separate defects, fixed together.

1. `services/notification-service/package.json` pinned `@types/node` to
   `18.19.0`. That directly violates CLAUDE.md's toolchain floor, which mandates
   `^24.13.3` in every manifest -- my error when the manifest was written, and
   what perturbed the workspace's nested dependency tree in the first place.
   Now `^24.13.3` like every sibling.

2. `index-docs.ts` declared `let entries: Awaited<ReturnType<typeof fs.readdir>>`.
   `readdir` is overloaded, and `ReturnType` silently resolves to the LAST
   overload -- which under @types/node 24 is the buffer-encoding variant
   returning `Dirent<Buffer>[]`, so the `{ withFileTypes: true }` call that
   actually follows (a `Dirent<string>[]`) could not be assigned to it. Replaced
   with an explicit `Dirent[]`: it is what the code means and it is stable across
   typings versions.

Why this surfaced only here, when master is green on the same file: the lockfile
nests a stale `@types/node@18.19.0` under chat-service even though its manifest
asks for `^24.13.3`. npm 10 honours that stale entry, so chat-service compiles
against Node 18 typings where `Dirent` is not generic and the `ReturnType` trick
happens to work. CI's npm 11 correctly ignores a nested entry that cannot satisfy
the declared range and hands chat-service the root 24.13.3 it asked for --
exposing a latent error rather than introducing one. So master is green by
accident, against typings its own floor forbids.

Verified against BOTH typings, since the local and CI resolutions differ:
- pre-fix vs 24.13.3 -> reproduces all five CI errors exactly
- post-fix vs 24.13.3 -> tsc clean
- post-fix vs 18.19.0 -> tsc clean
- chat-service: 131 passed / 2 skipped, 24 suites
- notification-service: `tsc` clean and 35/35, under 24.13.3 as well as 18.19.0
  (checked so this change does not simply relocate the same latent bug)
- `npm ci` exits 0; the only lockfile delta is the mirrored range

Also gitignored `services/notification-service/dist/`, which was untracked build
output. Listed explicitly alongside chat-service rather than as `services/*/dist/`
because email-service's dist is deliberately tracked.

Not fixed here, deliberately: eight other workspaces (shared, billing-client,
portal-client, packages/{chat-client,feature-flags,security,i18n-translate},
services/email-service) carry the same stale nested `@types/node@18` against a
`^24.13.3` manifest. Each is a latent version-sensitive break of exactly this
kind. Reconciling the lockfile repo-wide is its own change, not something to bury
inside this PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* fix(e2e): billing post-prod synthetic must seed the account vault, not `authToken`

Third and last spec seeding the bare `authToken` key, found by sweeping the whole
tree rather than one directory — which is how the first two were missed. This one
sits at `tests/e2e/billing-invoices/`, outside `frontend/`, so neither the earlier
`frontend/tests/**` pass nor the `frontend/e2e/post-prod/**` pass covered it.

Like `live-smoke.spec.ts`, it boots today only because the account vault's
one-time legacy migration sweeps the bare key at startup. Depending on an upgrade
path as a write channel means this goes quietly red against production the day
that migration is retired — and as a post-production synthetic, quietly red is
the worst failure mode it has.

The vault literal is duplicated rather than imported: this Playwright project has
its own config and must not reach into the shell's source tree, the same reason
`WhiteLabelLoginCard.tsx` duplicates it. `frontend/tests/support/account-vault.ts`
pins the format for the specs that CAN import it.

After this, the only remaining bare-`authToken` write in the repository is
`frontend/src/__tests__/accounts.test.ts`, which writes it deliberately to prove
the legacy migration sweeps it — the one place the bare key SHOULD appear.

Verified: frontend vitest 136/136 across 19 files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

* chore: untrack the generated playwright-report-prod artifact

`frontend/playwright-report-prod/index.html` is a 516K generated Playwright HTML
report that I committed by accident in f5dd652 while verifying the e2e specs.
Master's #447 added `**/playwright-report-prod/` to .gitignore, but gitignore does
not apply to already-tracked files, so it had to be removed from the index
explicitly. Confirmed ignored afterwards.

Two pre-existing artifacts are deliberately left alone because they are tracked on
master, not introduced here: `frontend/playwright-report/index.html` and
`frontend/test-results/{.last-run.json,auth-simple-success.png}`. Untracking
master's files is not this PR's business.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KynhzG6wKxUnR8KWwK8un

---------

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

3 participants