Files
pig/apps/api/test/read-governance.test.ts
karti 13dec6b4b8
CI / verify (push) Successful in 3m45s
CI / publish (push) Has been skipped
Rebuild the shell, add Calendar and Learn, and govern reads
Seven parallel agents and an adversarial verification pass. The three things
worth knowing before reading the diff:

RBAC WAS ALREADY BUILT. docs/build-plan.md marks F2 and F3 outstanding and is
stale — packages/core/src/permissions.ts and lib/mutation.ts shipped long ago.
So this does not rebuild them; it closes the gaps an audit found. The big one
is that reads were entirely ungoverned: every GET was "any authenticated
member", so a junior demand rep and a research contractor could both pull
per-block supplier cost and break-even prices from /api/capacity/margin, and
every contract's negotiated terms. For a company whose margin is the business,
that was the hole that mattered. Adds book:read / economics:read / team:read,
a readGuard middleware, and a `viewer` role below member.

THE BUTTON AND THE 403 DISAGREED — the exact thing F3 said must never happen.
Contracts.tsx never called can() at all, so its save button was always enabled
against a server requiring contract:sign; Capacity.tsx gated commitment
creation on deal:write/demand while the server wanted commitment:write/supply.

POST /api/activities was the one write bypassing executeMutation: no capability
check, and any member could mutate accounts.lastActivityAt as a side effect.
It is now a proper mutation() behind activity:write.

The shell becomes three panes — a collapsible shadcn sidebar with an account
switcher on the Piggy accent, a header with real search, and Piggy docked to
the right, page-aware and persistent across navigation. The phone keeps its
bottom tab bar, which is the thing this product already beat trycompai/crm on,
and gains the sidebar as a sheet.

Calendar is a projection over thirteen dated sources rather than a new table,
because a table would duplicate dates that already live on contracts, deals and
commitments and would drift — and one ledger answering the question is the
whole argument. It surfaces export_authorizations and compliance_artifacts,
which had indexed expires_at columns, schema comments saying they must be
alerted on, and no read endpoint or UI anywhere.

Learn carries two tracks. Concepts are members-only; the platform track can be
opened with a share code by someone with no account. The code mints a scoped
learn-only token and never a Principal — every route here resolves a principal
and then checks capabilities, so a principal-minting code would be one missing
check away from leaking the book. "Only platform-track rows may be code-visible"
is a database CHECK constraint as well as a write-path rule, and a test asserts
a valid learn token still gets 401 on /api/dashboard, /api/accounts and
/api/contracts — the same invariant scripts/deploy.sh refuses to ship without.

CD becomes tag-to-ship. CI publishes an image to the Gitea registry on a
release-* tag and cloud-2 pulls it, so no credential on the shared runner can
execute anything on production — by construction rather than by policy. Both
halves of deploy.sh's original rule survive: nothing on the runner reaches the
host, and a human still decides when it ships. deploy.sh gains a rollback and a
public-origin check, and PIG_IMAGE now reaches compose through `sudo env`,
without which sudo's env_reset silently resolved every release to pig:local.

Tests 141 -> 261.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-13 15:02:48 -07:00

198 lines
8.8 KiB
TypeScript

/**
* That reads are governed, and that the guard actually runs.
*
* Two separate risks. The policy could be wrong — a research contractor let
* near supplier cost — and that is what the first suite checks. Or the policy
* could be right and never execute, because Hono runs matched handlers in
* registration order and a guard mounted after its handler is inert. That
* second failure produces no error, no warning and a 200, which is exactly the
* shape of the bug being fixed, so it is checked separately and explicitly.
*/
import { strict as assert } from 'node:assert';
import { readdirSync, readFileSync } from 'node:fs';
import { join } from 'node:path';
import { describe, it } from 'node:test';
import type { Team, TeamRole } from '@pig/core';
import { Hono } from 'hono';
import { AuthError, type Principal } from '../src/lib/auth';
import { apiError, type ApiEnv } from '../src/lib/mutation';
import { createReadGuardRoutes, READ_RULES } from '../src/routes/read-guards';
import { principal as makePrincipal } from './helpers/principal';
/** The app's own error mapping, reproduced so a 403 here means a 403 there. */
function guardedApp(principal: Principal, mountGuardsFirst = true) {
const app = new Hono<ApiEnv>();
app.use('*', async (context, next) => {
context.set('principal', principal);
await next();
});
const handlers = new Hono<ApiEnv>();
for (const rule of READ_RULES) handlers.on(rule.method, rule.path, (c) => c.json({ ok: true }));
if (mountGuardsFirst) {
app.route('/', createReadGuardRoutes());
app.route('/', handlers);
} else {
app.route('/', handlers);
app.route('/', createReadGuardRoutes());
}
app.onError((error, c) =>
error instanceof AuthError
? c.json(apiError(error.code, error.message), error.status)
: c.json({ error: 'Internal error' }, 500),
);
return app;
}
function on(team: Team, role: TeamRole): Principal {
return makePrincipal({ teams: [{ team, role }] });
}
async function statusFor(principal: Principal, rule: (typeof READ_RULES)[number]) {
const path = rule.path.replace(':id', '00000000-0000-4000-8000-000000000001');
const response = await guardedApp(principal).request(path, {
method: rule.method,
...(rule.method === 'POST'
? { headers: { 'content-type': 'application/json' }, body: '{}' }
: {}),
});
return response.status;
}
describe('read policy', () => {
it('denies every governed read to someone on no team', async () => {
const stranger = makePrincipal({ teams: [] });
for (const rule of READ_RULES) {
assert.equal(await statusFor(stranger, rule), 403, `${rule.method} ${rule.path}`);
}
});
it('admits every governed read to a platform admin', async () => {
const admin = makePrincipal({ isPlatformAdmin: true, teams: [] });
for (const rule of READ_RULES) {
assert.equal(await statusFor(admin, rule), 200, `${rule.method} ${rule.path}`);
}
});
/**
* The case the audit named: a research contractor and a demand rep seeing
* supplier cost economics identically. They must now differ, and only on the
* economics rules — research still reads the book.
*/
it('splits research off the economics rules and nothing else', async () => {
const researcher = on('research', 'lead');
for (const rule of READ_RULES) {
const expected = rule.capability === 'economics:read' ? 403 : 200;
assert.equal(await statusFor(researcher, rule), expected, `${rule.method} ${rule.path}`);
}
});
it('gives a viewer the book and the roster but not the cost side', async () => {
const viewer = on('demand', 'viewer');
for (const rule of READ_RULES) {
const expected = rule.capability === 'economics:read' ? 403 : 200;
assert.equal(await statusFor(viewer, rule), expected, `${rule.method} ${rule.path}`);
}
});
it('admits a commercial member to everything, cost included', async () => {
const seller = on('demand', 'member');
for (const rule of READ_RULES) {
assert.equal(await statusFor(seller, rule), 200, `${rule.method} ${rule.path}`);
}
});
it('refuses a write-only credential even where the person qualifies', async () => {
const writeOnly = makePrincipal({ via: 'api_key', scopes: ['write'] });
const response = await guardedApp(writeOnly).request('/api/capacity/margin');
assert.equal(response.status, 403);
assert.equal(((await response.json()) as { code: string }).code, 'insufficient_scope');
});
});
describe('the guard has to be mounted before the handler', () => {
it('runs when registered first', async () => {
const response = await guardedApp(on('research', 'lead'), true).request('/api/capacity/margin');
assert.equal(response.status, 403);
});
/**
* Not a test of desired behaviour — a test of the trap. If this ever starts
* returning 403, Hono's dispatch order changed and the warning comment in
* read-guards.ts can be deleted. Until then, the mount position in
* `createApp` is load-bearing and this records why.
*/
it('is silently inert when registered after', async () => {
const response = await guardedApp(on('research', 'lead'), false).request('/api/capacity/margin');
assert.equal(response.status, 200);
});
});
/**
* Nothing stops a future GET being added without a row in READ_RULES, so this
* reads the routing source and insists that every `/api` GET is either
* governed or listed below with a reason. It is a coarse regex over source
* text and that is deliberate: a cleverer check would need the app running,
* and a check that is hard to run is a check that gets deleted.
*/
describe('no read escapes the table', () => {
/** Reads whose own handler authorises them, or which must stay open. */
const DELIBERATELY_UNGOVERNED: Readonly<Record<string, string>> = {
'/api/health': 'Liveness, for load balancers. Unauthenticated by design.',
'/api/config': 'Public front-end configuration; contains no secret.',
'/api/me': 'Your own identity. Gating it would hide the reason you are gated.',
'/api/me/profile': 'Your own profile row.',
'/api/api-keys': 'Guarded by requireApiKeyManagement, which also bars API keys.',
'/api/admin/settings': 'settings:admin, enforced in admin-settings.ts.',
'/api/admin/invites': 'settings:admin, enforced in admin-settings.ts.',
'/api/admin/members': 'settings:admin, enforced in admin-settings.ts.',
'/api/admin/integrations': 'settings:admin, enforced in integration-settings.ts.',
'/api/piggy/status': 'Whether the assistant is switched on; carries no book data.',
'/api/imports/config': 'data:import, enforced by the router middleware.',
'/api/imports/google/status': 'integration:connect, enforced by the router middleware.',
'/api/imports/google/files': 'data:import, enforced by the router middleware.',
'/api/imports/google/spreadsheets/:id/sheets': 'data:import, enforced by the router middleware.',
'/api/imports/notion/status': 'integration:connect, enforced by the router middleware.',
'/api/imports/notion/connections/:id/data-sources': 'integration:connect, ditto.',
'/api/integrations/hubspot/oauth/callback': 'OAuth redirect; verifies its own state.',
'/api/integrations/hubspot/connections': 'settings:admin, enforced in hubspot.ts.',
'/api/integrations/slack/channel-links': 'Channel wiring, not book data.',
'/api/integrations/buzz/channel-links': 'Channel wiring, not book data.',
'/api/calendar': 'Owned by the calendar track; gated in calendar.ts.',
'/api/calendar/entries': 'Owned by the calendar track; gated in calendar.ts.',
// Landed while this table was being written and carries its own access
// code rather than a capability. Listed so the check stays green, not
// because the arrangement has been reviewed — the learn track owns it.
'/api/learn': 'Owned by the learn track; gated by its own access code.',
'/api/learn/access-code': 'Owned by the learn track; gated by its own access code.',
};
it('has a row, or a stated reason, for every GET', () => {
const root = join(import.meta.dirname, '..', 'src');
const files = [
join(root, 'app.ts'),
...readdirSync(join(root, 'routes'))
.filter((name) => name.endsWith('.ts'))
.map((name) => join(root, 'routes', name)),
];
const governed = new Set(READ_RULES.filter((rule) => rule.method === 'GET').map((r) => r.path));
const found = new Set<string>();
for (const file of files) {
const source = readFileSync(file, 'utf8');
for (const match of source.matchAll(/\.get\(\s*'(\/api\/[^']*)'/g)) found.add(match[1]!);
}
const ungoverned = [...found].filter(
(path) => !governed.has(path) && !(path in DELIBERATELY_UNGOVERNED),
);
assert.deepEqual(
ungoverned,
[],
`these reads are ungoverned — add a READ_RULES row or a stated reason:\n${ungoverned.join('\n')}`,
);
});
});