Skip to content

feat(core): managementApi.proxy.scopes and a tenant-isolation suite on real Postgres (F6) - #685

Open
seanspeaks wants to merge 5 commits into
claude/internal-mcp-05c-credential-scopingfrom
claude/internal-mcp-06-proxy-scopes
Open

seanspeaks wants to merge 5 commits into
claude/internal-mcp-05c-credential-scopingfrom
claude/internal-mcp-06-proxy-scopes

Conversation

@seanspeaks

@seanspeaks seanspeaks commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This is work package F6 of the internal-MCP build plan (ADR-056 §3.5). It is stacked on #684 (05c); retarget it to the next branch down once that merges.

managementApi.proxy.scopes

  • Setting. Defaults to ['individual']. It is validated in resolveManagementApiConfig, which loadAppDefinition now calls too.
  • Bad values fail at load. An unknown value, 'global', a non-list, an empty list or a repeated value throws ManagementApiConfigError (MANAGEMENT_API_CONFIG_INVALID), even when the proxy is off. It never falls back to the default.
  • Schema. scopes is added to the app-definition JSON schema in packages/schemas.

Proxy guard

  • Where it runs. In the use case, ExecuteEntityProxyRequest.assertProxyable, not the handler.
  • Same rule as the lists. It reads { scope, access } from GetEntityModuleForUser.describe, the resolver the v2 lists use, so there is one ownership rule, not two.
  • Responses, checked in this order:
    • Not visible in the active org: 404, unchanged.
    • Visible but without use access: 403 PROXY_NOT_ALLOWED, details.reason: 'access'.
    • Scope not in proxy.scopes: 403 PROXY_NOT_ALLOWED, details.reason: 'scope'.
  • "Use access" means use or manage, in one helper, grantsUse. An entity's owner holds manage, so a strict === 'use' would stop owners proxying their own entities.

Fix found by the new suite

  • The bug. A missing entity id answered 500 on every database, while another tenant's id answered 404. That difference told a caller which ids exist.
  • The fix. A new modules/repositories/entity-not-found.js, modelled on integration-not-found.js, is thrown by all four module repositories and mapped to 404 by GetEntityModuleForUser. This affects every v2 entity route.
  • Not changed. Other callers of findEntityById (get-module, module-factory, test-module-auth, …) behave as before.

Tenant-isolation suite, real PostgreSQL

File: packages/core/management-api/v2/tenant-isolation.postgres.test.js.

Setup. It applies the real migrations and asserts it is talking to PostgreSQL. Requests go through real HS256 adopter-JWT auth, the real router, use cases and Prisma repositories. Only the API module's _proxyRequest is stubbed, so no upstream is called. The users are A and B; the orgs are X and Y.

What it proves:

  • A cannot list, read, proxy or bind B's entities, and neither can an org admin.
  • A, active in X, cannot reach A's own Y entities, and can when active in Y.
  • Organization entities get 403 scope by default, even for an admin. With organization in scopes, a plain member with use access can proxy through them. They are never visible from another org.
  • individualAcross: 'all':
    • the entity is stored with a null organizationId;
    • its owner can use it in X and Y, and nobody else can;
    • an integration using it belongs to the org it was created in.

When it runs. Only when FRIGG_TEST_POSTGRES_URL is set; it truncates all tables before each test. Without the variable it is skipped, with a printed warning, so the default unit run doesn't gain a suite that always fails.

Docs and types

  • docs/guides/entity-scopes.md: F6 removed from the Deferred list.
  • Management API notes in packages/core/CLAUDE.md and the root CLAUDE.md.
  • Types: ManagementApiConfig, ManagementApiProxyConfig (whose scopes exclude 'global') and ManagementApiConfigError, which index.js now exports.

Test plan

  • Core suite, using the no-op global setup:
    • Before: 14 failing suites and 61 failing tests, identical to the 05c base; 3449 passing.
    • After: the same failures, by name and per-suite count; 3494 passing.
  • Isolation suite against a real PostgreSQL 16: 11 of 11 pass.
  • packages/schemas: 207 of 207 pass.
  • ESLint on the changed files: no errors.
  • Mutation testing: 19 mutants, all killed.
    • Killed by both the unit tests and the Postgres suite: removing the scope check; grantsUse accepting only manage; a wider default scope list; ignoring the setting; no 404 for a missing row; no visibility check on load; ignoring 'all'; dropping the org match.
    • The rest are killed by the unit tests.
  • Not verified: CI's own Postgres job, if there is one.

Notes for the reviewer

  • Nothing in the code produces view access yet: the org-admin view of members' entities is still deferred. The access refusal is therefore reachable only in unit tests.
  • A separate fix round for findings across F3–05c is in progress. It will be merged into this branch when it lands.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6


Generated by Claude Code


Note

High Risk
Changes authentication-adjacent tenant boundaries for the entity proxy and normalizes missing-entity responses across v2 entity routes; misconfigured proxy.scopes now prevents app startup.

Overview
Implements F6 (ADR-056 §3.5): configurable entity-proxy scopes, stricter proxy authorization, and a real-PostgreSQL tenant-isolation suite.

managementApi.proxy.scopes defaults to ['individual'] and is resolved in resolveManagementApiConfig, which loadAppDefinition now invokes so invalid values fail startup with ManagementApiConfigError (MANAGEMENT_API_CONFIG_INVALID) even when the proxy is disabled. The app-definition JSON schema and TypeScript types document the setting; global is never allowed.

Entity proxy (ExecuteEntityProxyRequest.assertProxyable) reuses the same { scope, access } as v2 lists via GetEntityModuleForUser.describe: invisible entities stay 404; visible callers without use or manage get 403 PROXY_NOT_ALLOWED (reason: 'access'); scopes outside the allow-list get 403 (reason: 'scope'). Organization entities require an explicit opt-in to organization in scopes.

Missing entity IDs no longer leak existence via 500: all module repositories throw through entity-not-found.js, and GetEntityModuleForUser maps that to the same 404 as cross-tenant access (affects v2 entity routes broadly).

Adds tenant-isolation.postgres.test.js (runs only when FRIGG_TEST_POSTGRES_URL is set) for list/read/proxy/bind isolation, scope gating, and individualAcross: 'all'. Docs and OpenAPI descriptions are updated accordingly.

Reviewed by Cursor Bugbot for commit a13fdab. Bugbot is set up for automated code reviews on this repo. Configure here.

📦 Published PR as canary version: 2.0.0--canary.685.a13fdab.0

✨ Test out this PR locally via:

npm install @friggframework/admin-scripts@2.0.0--canary.685.a13fdab.0
npm install @friggframework/core@2.0.0--canary.685.a13fdab.0
npm install @friggframework/devtools@2.0.0--canary.685.a13fdab.0
npm install @friggframework/eslint-config@2.0.0--canary.685.a13fdab.0
npm install @friggframework/prettier-config@2.0.0--canary.685.a13fdab.0
npm install @friggframework/schemas@2.0.0--canary.685.a13fdab.0
npm install @friggframework/serverless-plugin@2.0.0--canary.685.a13fdab.0
npm install @friggframework/test@2.0.0--canary.685.a13fdab.0
npm install @friggframework/ui@2.0.0--canary.685.a13fdab.0
# or 
yarn add @friggframework/admin-scripts@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/core@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/devtools@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/eslint-config@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/prettier-config@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/schemas@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/serverless-plugin@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/test@2.0.0--canary.685.a13fdab.0
yarn add @friggframework/ui@2.0.0--canary.685.a13fdab.0

…proxy (ADR-056 §3.5)

- `managementApi.proxy.scopes`, default ['individual'], lists the entity
  scopes the proxy accepts. Validated in resolveManagementApiConfig, which
  loadAppDefinition now calls: an unknown scope, 'global', a non-list, an
  empty list or a duplicate throws ManagementApiConfigError
  (MANAGEMENT_API_CONFIG_INVALID) at load, even with the proxy off. Added to
  the app-definition JSON schema.
- ExecuteEntityProxyRequest checks, after the existing visibility 404, the
  description the v2 lists return (GetEntityModuleForUser.describe): access
  must grant use (`use` or `manage`; `view` is refused), and the scope must
  be in proxy.scopes. Both refusals are 403 PROXY_NOT_ALLOWED with
  details.reason 'access' or 'scope'; an entity not visible in the active
  organization stays 404.
- grantsUse(access) in scope-visibility is the one rule for "use access".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
…ot a 500

Every module repository adapter throws `Entity <id> not found` for a
missing row, but GetEntityModuleForUser.loadOwnedEntity mapped only
malformed ids to 404 and rethrew the rest. So on v2 (GET/DELETE
/entities/:id, its options and test-auth routes, and the proxy) an id
that does not exist answered 500 while another tenant's answered 404:
the status code said which ids exist.

Found by the real-Postgres isolation suite; the unit tests mocked
findEntityById as resolving null, which no adapter does.

entity-not-found.js holds the error and its one predicate, as
integration-not-found.js does for integrations; the adapters build it,
the use case matches it, and the message is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
….6, F6)

Real adopter-JWT auth (HS256), the real v2 router, use cases and Prisma
repositories against PostgreSQL; the migrations are applied first and
every table is truncated before each test. Only the upstream API behind
the proxy is a stand-in. Users A and B, organizations X and Y:

- A cannot list, read, proxy or bind B's entities (an org admin neither);
  a missing id is the same 404 as a foreign one.
- A active in X cannot reach A's own Y entities, and can in Y.
- Organization entities: 403 reason 'scope' under the default
  proxy.scopes, even for an admin; a plain member with use access
  proxies once the app opts in; never visible from another organization;
  scopes ['organization'] refuses individual entities.
- individualAcross 'all': stored with no organization, usable by its owner
  in every organization, by nobody else; an integration using it belongs
  to the organization it was created in.

Runs when FRIGG_TEST_POSTGRES_URL names a database it may wipe;
otherwise skipped, and it says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
- docs/guides/entity-scopes.md: the proxy's three checks (404 visibility,
  403 access, 403 scope), managementApi.proxy.scopes and its load-time
  validation, the missing-id 404, how to run the Postgres suite; F6
  removed from Deferred.
- packages/core/CLAUDE.md: Management API v2 notes; root CLAUDE.md: one
  line on proxy access and scopes.
- types: ManagementApiConfig / ManagementApiProxyConfig on AppDefinition
  (scopes excludes 'global'), ManagementApiConfigError, which index.js
  now exports beside OwnershipConfigError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
@seanspeaks seanspeaks added release Create a release when this pr is merged prerelease This change is available in a prerelease. labels Oct 16, 2026 — with Claude

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

Copy link
Copy Markdown
Contributor Author

build (22.x) is red, and the failures come from the base branch, not this PR.

Every failure is in @friggframework/devtools: 33 suites and 152 tests. The failing suites include frigg-cli, management-ui, infrastructure/template-generation and build-prisma-layer, and this PR changes none of that code.

The base PR, #684 (job 116782953973), fails the same 33 suites by name with the same counts (33 failed, 1 skipped, 70 passed and 152 failed, 10 skipped, 1713 passed). The rest of the job passes here: packages/schemas goes from 199 to 207 tests, the new proxy.scopes schema tests, all green.

I haven't ported a fix, because none exists yet. The devtools failures were already there before this stack; I'll port a fix into this PR once one lands.


Generated by Claude Code

…/internal-mcp-06-proxy-scopes

Brings in the stack fix round (deprecated module-definitions wrapper, DocumentDB
ownership drains, v1 POST /api/entity owner and admin gate, membership-role
fallback). Conflict in types/modules/index.d.ts: kept both
ManagementApiConfigError (F6) and the deprecated
getModulesDefinitionFromIntegrationClasses (F3).

Core suite: 14 suites / 61 tests failing, the same names as 05c; 3534 passing.

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

Copy link
Copy Markdown

@cursor

cursor Bot commented Oct 24, 2026

Copy link
Copy Markdown

Frigg 2.0.1 review (Stack A) — COMMENT only

Verdict (would): changes needed

Proxy scope gate (managementApi.proxy.scopes, default ['individual']), use-access check, and header policy (Authorization / cookie / Host / forwarding rejected; module allow-list cannot override) look strong. Path normalization and redirect re-checks are solid. The isolation suite is the right shape — but it does not run in CI.

Blocking

  1. Real-Postgres tenant-isolation suite never runs in CI
    • management-api/v2/tenant-isolation.postgres.test.js — describe.skip unless FRIGG_TEST_POSTGRES_URL
    • .github/workflows/frigg-ci.js.yml — npm run test:all only; no Postgres service, no env
    • core jest --passWithNoTests → skipped suite = green
      Failure: Ownership/proxy/tenant isolation regressions merge while CI stays green; the suite’s own warning admits skip ≠ pass.
      Fix: Wire a CI job with Postgres + FRIGG_TEST_POSTGRES_URL that fails if this file is skipped (dedicated script or assert env in CI).

Should-fix

  1. Named base URLs / function baseUrl SSRF residual — proxy.baseUrl may be a function of api (tenant shop/instance URL). joinProxyUrl enforces https and path containment under that base, but no host allow-list / private-IP block. Until named bases (feat(core): several named proxy base URLs per module (F8) #690), disallow function baseUrl that returns non-constant hosts, or document that modules must not derive base URL from tenant-controlled fields.
  2. Nit: Consider rejecting x-frigg-* on upstream proxy requests for defense-in-depth.

No formal approve / request-changes on this review.

const path = require('node:path');
const { execFileSync } = require('node:child_process');

const describePostgres = POSTGRES_URL ? describe : describe.skip;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blocking — tenant-isolation suite is skipped in default CI

describePostgres = POSTGRES_URL ? describe : describe.skip, and .github/workflows/frigg-ci.js.yml runs only npm run test:all with no Postgres service and no FRIGG_TEST_POSTGRES_URL. Core Jest uses --passWithNoTests, so a fully skipped suite is green.

Scenario: A regression in ownership / proxy scopes / cross-org 404s merges while CI stays green. The warning on lines 41–42 admits skip ≠ pass — please wire a CI job that sets FRIGG_TEST_POSTGRES_URL (and fails the job if this file would be skipped).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

prerelease This change is available in a prerelease. release Create a release when this pr is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants