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>
This commit is contained in:
+67
-10
@@ -24,12 +24,17 @@ import type { Database } from '@pig/db';
|
||||
import { apiKeys, teamMemberships, users } from '@pig/db';
|
||||
import {
|
||||
permissionGranted,
|
||||
resolvePermissionGrants,
|
||||
type Capability,
|
||||
resolveReadPermissionGrants,
|
||||
resolveWritePermissionGrants,
|
||||
roleMeets,
|
||||
TEAM_CAPABILITY_RULES,
|
||||
type GlobalCapability,
|
||||
type PermissionGrant,
|
||||
type ReadCapability,
|
||||
type Team,
|
||||
type TeamCapability,
|
||||
type TeamRole,
|
||||
type WriteCapability,
|
||||
} from '@pig/core';
|
||||
import { createHash, timingSafeEqual } from 'node:crypto';
|
||||
import type { Config } from './config';
|
||||
@@ -231,8 +236,7 @@ export function hasTeamAccess(
|
||||
if (principal.isPlatformAdmin) return true;
|
||||
const membership = principal.teams.find((t) => t.team === team);
|
||||
if (!membership) return false;
|
||||
const rank: Record<TeamRole, number> = { member: 0, lead: 1, admin: 2 };
|
||||
return rank[membership.role] >= rank[minimumRole];
|
||||
return roleMeets(membership.role, minimumRole);
|
||||
}
|
||||
|
||||
export function requireScope(principal: Principal, scope: string): void {
|
||||
@@ -240,13 +244,22 @@ export function requireScope(principal: Principal, scope: string): void {
|
||||
throw new AuthError(`This credential lacks the '${scope}' scope.`, 403, 'insufficient_scope');
|
||||
}
|
||||
|
||||
/** Effective grants include credential scope, not merely the owner's roles. */
|
||||
/**
|
||||
* Effective grants include credential scope, not merely the owner's roles.
|
||||
*
|
||||
* Read and write scopes are filtered separately. Before read capabilities
|
||||
* existed a read-only key resolved to no grants at all, which was right then
|
||||
* and would now be wrong: it would tell `/api/me` that a read-only agent may
|
||||
* not read, and the browser would grey out a page the server happily serves.
|
||||
*/
|
||||
export function effectivePermissions(principal: Principal): PermissionGrant[] {
|
||||
if (!principal.scopes.includes('write')) return [];
|
||||
return resolvePermissionGrants(principal);
|
||||
const grants: PermissionGrant[] = [];
|
||||
if (principal.scopes.includes('read')) grants.push(...resolveReadPermissionGrants(principal));
|
||||
if (principal.scopes.includes('write')) grants.push(...resolveWritePermissionGrants(principal));
|
||||
return grants;
|
||||
}
|
||||
|
||||
export function requireCapability(principal: Principal, capability: Capability): void;
|
||||
export function requireCapability(principal: Principal, capability: GlobalCapability): void;
|
||||
export function requireCapability(
|
||||
principal: Principal,
|
||||
capability: TeamCapability,
|
||||
@@ -254,14 +267,58 @@ export function requireCapability(
|
||||
): void;
|
||||
export function requireCapability(
|
||||
principal: Principal,
|
||||
capability: Capability,
|
||||
capability: WriteCapability,
|
||||
team?: Team,
|
||||
): void {
|
||||
requireScope(principal, 'write');
|
||||
if (permissionGranted(resolvePermissionGrants(principal), capability, team)) return;
|
||||
if (permissionGranted(resolveWritePermissionGrants(principal), capability, team)) return;
|
||||
throw new AuthError(
|
||||
`This principal lacks the '${capability}' capability${team ? ` for ${team}` : ''}.`,
|
||||
403,
|
||||
'insufficient_permission',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* "May they do this on *some* team?"
|
||||
*
|
||||
* Separate from `requireCapability` and deliberately harder to type by
|
||||
* accident. Passing no team to the old `requireCapability` silently meant this
|
||||
* — which is how a research-team admin could bulk-import demand deals — so the
|
||||
* overloads above now refuse it and every remaining any-team check has to say
|
||||
* so in its own name. Use it only where no team is knowable yet: listing the
|
||||
* spreadsheets in someone's Drive, before an entity has been chosen. The
|
||||
* moment the target is known, go back to `requireCapability` with its team.
|
||||
*/
|
||||
export function requireAnyTeamCapability(
|
||||
principal: Principal,
|
||||
capability: TeamCapability,
|
||||
): void {
|
||||
requireScope(principal, 'write');
|
||||
const grants = resolveWritePermissionGrants(principal);
|
||||
for (const team of TEAM_CAPABILITY_RULES[capability].teams) {
|
||||
if (permissionGranted(grants, capability, team)) return;
|
||||
}
|
||||
throw new AuthError(
|
||||
`This principal lacks the '${capability}' capability on any team.`,
|
||||
403,
|
||||
'insufficient_permission',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads are governed too. The 'read' scope is checked rather than 'write'
|
||||
* because a read-only API key is exactly the credential this must admit.
|
||||
*/
|
||||
export function requireReadCapability(
|
||||
principal: Principal,
|
||||
capability: ReadCapability,
|
||||
): void {
|
||||
requireScope(principal, 'read');
|
||||
if (permissionGranted(resolveReadPermissionGrants(principal), capability)) return;
|
||||
throw new AuthError(
|
||||
`This principal lacks the '${capability}' capability.`,
|
||||
403,
|
||||
'insufficient_permission',
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ActivityType, GlobalCapability, Team, TeamCapability } from '@pig/core';
|
||||
import { isTeamCapability } from '@pig/core';
|
||||
import type { Database } from '@pig/db';
|
||||
import { activities } from '@pig/db';
|
||||
import type { Context, Handler } from 'hono';
|
||||
@@ -55,9 +56,18 @@ export interface MutationActivity {
|
||||
meta?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* `'self'` is for the one write whose own row IS the audit event: logging an
|
||||
* activity. Inserting an audit row about it would double every synced call in
|
||||
* the feed. It is a literal rather than an omitted field so that audit can
|
||||
* never be skipped by forgetting to write one — the type still demands an
|
||||
* answer, and `'self'` is a visible, greppable claim.
|
||||
*/
|
||||
export type MutationAudit = MutationActivity | 'self';
|
||||
|
||||
export interface MutationResult<Result> {
|
||||
data: Result;
|
||||
activity: MutationActivity;
|
||||
activity: MutationAudit;
|
||||
}
|
||||
|
||||
interface MutationContext<Input> {
|
||||
@@ -80,11 +90,15 @@ function enforcePermission(principal: Principal, permission: PermissionRequireme
|
||||
permission.authorize(principal);
|
||||
return;
|
||||
}
|
||||
if (permission.capability === 'settings:admin') {
|
||||
requireCapability(principal, permission.capability);
|
||||
// Discriminated by the capability itself rather than by a hard-coded
|
||||
// 'settings:admin' check, which quietly sent any future global capability
|
||||
// down the team-scoped branch with an undefined team — the "passes on any
|
||||
// team" bug, reintroduced by omission.
|
||||
if (isTeamCapability(permission.capability)) {
|
||||
requireCapability(principal, permission.capability, permission.team as Team);
|
||||
return;
|
||||
}
|
||||
requireCapability(principal, permission.capability, permission.team);
|
||||
requireCapability(principal, permission.capability as GlobalCapability);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -131,13 +145,15 @@ export async function executeMutation<Schema extends ZodTypeAny, Result>(
|
||||
};
|
||||
const result = await definition.mutate(context);
|
||||
|
||||
await tx.insert(activities).values({
|
||||
...result.activity,
|
||||
actorUserId: principal.userId,
|
||||
actorAgent: principal.via === 'api_key' ? 'agent' : null,
|
||||
source: principal.via === 'api_key' ? 'agent' : 'manual',
|
||||
occurredAt: now,
|
||||
});
|
||||
if (result.activity !== 'self') {
|
||||
await tx.insert(activities).values({
|
||||
...result.activity,
|
||||
actorUserId: principal.userId,
|
||||
actorAgent: principal.via === 'api_key' ? 'agent' : null,
|
||||
source: principal.via === 'api_key' ? 'agent' : 'manual',
|
||||
occurredAt: now,
|
||||
});
|
||||
}
|
||||
return result.data;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
/**
|
||||
* Authorisation for reads.
|
||||
*
|
||||
* The write path has had one chokepoint since F2 — `executeMutation` — and
|
||||
* reads had none. Every GET was "any authenticated member", so a research
|
||||
* contractor and a demand lead saw supplier cost per GPU-hour, break-even
|
||||
* price and the full negotiated terms of every contract identically. For a
|
||||
* company whose margin is the product, that was the hole that mattered.
|
||||
*
|
||||
* This is the reading half of the same chokepoint. It is thin on purpose:
|
||||
* capability in, middleware out, and the AuthError it throws is mapped to HTTP
|
||||
* by `app.onError` exactly as the write path's is, so a read denial and a write
|
||||
* denial are indistinguishable in shape to a client.
|
||||
*
|
||||
* `growth.ts` had the shape of this already but keyed on API-key *scope*, which
|
||||
* answers "is this credential allowed to read anything?" and not "is this
|
||||
* person allowed to read *this*". Scope is a property of the credential; the
|
||||
* capability is a property of the person. Both are checked here.
|
||||
*/
|
||||
import type { ReadCapability } from '@pig/core';
|
||||
import type { MiddlewareHandler } from 'hono';
|
||||
import { requireReadCapability } from './auth';
|
||||
import type { ApiEnv } from './mutation';
|
||||
|
||||
export function readGuard(capability: ReadCapability): MiddlewareHandler<ApiEnv> {
|
||||
return async (context, next) => {
|
||||
requireReadCapability(context.get('principal'), capability);
|
||||
await next();
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user