audit: Tier 2/3/4 batch — reports math, portal copy, authz escalation guard
Tier 2.2: revenue PDF totalCompleted now filters on outcome='won' — setInterestOutcome forces stage='completed' for every outcome (incl. lost + cancelled), so the stage-only filter was including those toward "TOTAL COMPLETED REVENUE". Tier 2.3: fetchPipelineData stageCounts adds the missing .groupBy() — without it Postgres rejects the SELECT (per-stage breakdown was broken or coercing to ELSE-stage row). Tier 2.4: hot-deals widget rank ladder fixed two stage-name typos — 'in_comms' → 'in_communication', 'deposit_10' → 'deposit_10pct'. Both stages were collapsing to the ELSE 0 branch server-side AND rendering raw enum to the user in hot-deals-card.tsx. Tier 3.2: portal /portal/interests no longer renders raw enum to clients. New PORTAL_SIGNING_LABELS table maps every EOI/contract status to plain English (e.g. "waiting_for_signatures" → "Waiting for signatures"). Tier 4.1 (CRITICAL): permission-overrides PUT now requires caller- superset on every `true` write. Admins with only `admin.manage_users` could previously grant other users leaves they don't hold themselves (permanently_delete_clients, system_backup). Super-admins bypass. Tier 4.4: search graph-expansion re-gates every merged bucket by the destination's view permission. A user with berths.view but no interests.view searching "A12" no longer sees interest rows surfaced via expansion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,4 +3,3 @@
|
||||
Items from the 33-agent audit that I deliberately left for you to decide on. Each one has the finding, why I parked it, and the proposed options.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -9,6 +9,32 @@ import { stageLabel, safeStage, type PipelineStage } from '@/lib/constants';
|
||||
|
||||
export const metadata: Metadata = { title: 'Interests' };
|
||||
|
||||
// Portal-friendly labels for signing-process status fields. The audit
|
||||
// caught raw enum leak ("waiting_for_signatures" with underscores) at
|
||||
// the client-facing surface. Map every known value to plain English;
|
||||
// fall back to a Title-Case rendering for any new states.
|
||||
const PORTAL_SIGNING_LABELS: Record<string, string> = {
|
||||
not_started: 'Not started',
|
||||
draft: 'Drafted',
|
||||
awaiting_them: 'Awaiting their signature',
|
||||
awaiting_me: 'Awaiting your signature',
|
||||
waiting_for_signatures: 'Waiting for signatures',
|
||||
partially_signed: 'Partially signed',
|
||||
sent: 'Sent for signing',
|
||||
signed: 'Signed',
|
||||
completed: 'Signed',
|
||||
expired: 'Expired',
|
||||
cancelled: 'Cancelled',
|
||||
rejected: 'Rejected',
|
||||
};
|
||||
function portalSigningLabel(status: string): string {
|
||||
if (status in PORTAL_SIGNING_LABELS) return PORTAL_SIGNING_LABELS[status]!;
|
||||
return status
|
||||
.split('_')
|
||||
.map((p) => (p ? p[0]!.toUpperCase() + p.slice(1) : p))
|
||||
.join(' ');
|
||||
}
|
||||
|
||||
const STAGE_VARIANT: Record<PipelineStage, 'default' | 'secondary' | 'destructive' | 'outline'> = {
|
||||
open: 'secondary',
|
||||
details_sent: 'secondary',
|
||||
@@ -77,10 +103,10 @@ export default async function PortalInterestsPage() {
|
||||
</span>
|
||||
)}
|
||||
{interest.eoiStatus && (
|
||||
<span>EOI: {interest.eoiStatus.replace(/_/g, ' ')}</span>
|
||||
<span>EOI: {portalSigningLabel(interest.eoiStatus)}</span>
|
||||
)}
|
||||
{interest.contractStatus && (
|
||||
<span>Contract: {interest.contractStatus.replace(/_/g, ' ')}</span>
|
||||
<span>Contract: {portalSigningLabel(interest.contractStatus)}</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -181,6 +181,16 @@ export const PUT = withAuth(
|
||||
// here we drop unknown resources/actions so a malicious client
|
||||
// can't seed garbage keys that a future resolver might accidentally
|
||||
// honour.
|
||||
// CALLER-SUPERSET (authz-auditor CRITICAL): an admin with only
|
||||
// `admin.manage_users` previously could grant another user any
|
||||
// permission leaf — including ones they don't hold themselves
|
||||
// (e.g. `permanently_delete_clients`, `system_backup`). Require
|
||||
// every `true` write to be a leaf the caller already has.
|
||||
// Super-admins bypass (they hold all leaves by definition).
|
||||
const callerPerms = ctx.permissions as Record<
|
||||
string,
|
||||
Record<string, boolean>
|
||||
> | null;
|
||||
const sanitized: Record<string, Record<string, boolean>> = {};
|
||||
for (const [resource, actions] of Object.entries(overrides)) {
|
||||
const allowed = ALLOWED_RESOURCE_ACTIONS[resource];
|
||||
@@ -193,6 +203,14 @@ export const PUT = withAuth(
|
||||
`permission override for ${resource}.${action} must be true or false`,
|
||||
);
|
||||
}
|
||||
if (value === true && !ctx.isSuperAdmin) {
|
||||
const callerHas = callerPerms?.[resource]?.[action] === true;
|
||||
if (!callerHas) {
|
||||
throw new ForbiddenError(
|
||||
`You don't hold ${resource}.${action} yourself, so you can't grant it.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
cleaned[action] = value;
|
||||
}
|
||||
if (Object.keys(cleaned).length > 0) sanitized[resource] = cleaned;
|
||||
|
||||
@@ -10,22 +10,22 @@ type AlertStatus = 'open' | 'dismissed' | 'resolved';
|
||||
// page uses.
|
||||
export const GET = withAuth(
|
||||
withPermission('admin', 'view_audit_log', async (req: NextRequest, ctx) => {
|
||||
const url = new URL(req.url);
|
||||
const status = (url.searchParams.get('status') ?? 'open') as AlertStatus;
|
||||
const url = new URL(req.url);
|
||||
const status = (url.searchParams.get('status') ?? 'open') as AlertStatus;
|
||||
|
||||
const rows = await listAlertsForPort(ctx.portId, {
|
||||
includeDismissed: status !== 'open',
|
||||
includeResolved: status !== 'open',
|
||||
});
|
||||
const rows = await listAlertsForPort(ctx.portId, {
|
||||
includeDismissed: status !== 'open',
|
||||
includeResolved: status !== 'open',
|
||||
});
|
||||
|
||||
// Filter to the requested status bucket so callers don't see overlap.
|
||||
const filtered = rows.filter((a) => {
|
||||
if (status === 'open') return !a.dismissedAt && !a.resolvedAt;
|
||||
if (status === 'dismissed') return Boolean(a.dismissedAt) && !a.resolvedAt;
|
||||
if (status === 'resolved') return Boolean(a.resolvedAt);
|
||||
return true;
|
||||
});
|
||||
// Filter to the requested status bucket so callers don't see overlap.
|
||||
const filtered = rows.filter((a) => {
|
||||
if (status === 'open') return !a.dismissedAt && !a.resolvedAt;
|
||||
if (status === 'dismissed') return Boolean(a.dismissedAt) && !a.resolvedAt;
|
||||
if (status === 'resolved') return Boolean(a.resolvedAt);
|
||||
return true;
|
||||
});
|
||||
|
||||
return NextResponse.json({ data: filtered });
|
||||
return NextResponse.json({ data: filtered });
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -23,13 +23,17 @@ interface HotDealsResponse {
|
||||
data: HotDeal[];
|
||||
}
|
||||
|
||||
// Local label map intentionally narrowed to the stages this widget
|
||||
// surfaces. Keys MUST match the canonical DB values (deposit_10pct +
|
||||
// in_communication) — the reporting audit caught typos that broke the
|
||||
// rank ladder server-side AND rendered raw enum to the user.
|
||||
const STAGE_LABELS: Record<string, string> = {
|
||||
contract_signed: 'Contract Signed',
|
||||
contract_sent: 'Contract Sent',
|
||||
deposit_10: 'Deposit 10%',
|
||||
deposit_10pct: 'Deposit 10%',
|
||||
eoi_signed: 'EOI Signed',
|
||||
eoi_sent: 'EOI Sent',
|
||||
in_comms: 'In Comms',
|
||||
in_communication: 'In Comms',
|
||||
details_sent: 'Details Sent',
|
||||
open: 'Open',
|
||||
completed: 'Completed',
|
||||
|
||||
@@ -195,14 +195,18 @@ export async function getBerthStatusDistribution(portId: string) {
|
||||
*/
|
||||
export async function getHotDeals(portId: string, limit = 5) {
|
||||
// Stage rank: bigger = closer to closing.
|
||||
// Reporting audit caught two stage-name typos: 'in_comms' and
|
||||
// 'deposit_10' don't exist in the DB enum — canonical values are
|
||||
// 'in_communication' and 'deposit_10pct'. Those two stages were
|
||||
// silently collapsing to the ELSE 0 branch.
|
||||
const rank = sql<number>`CASE ${interests.pipelineStage}
|
||||
WHEN 'completed' THEN 8
|
||||
WHEN 'contract_signed' THEN 7
|
||||
WHEN 'contract_sent' THEN 6
|
||||
WHEN 'deposit_10' THEN 5
|
||||
WHEN 'deposit_10pct' THEN 5
|
||||
WHEN 'eoi_signed' THEN 4
|
||||
WHEN 'eoi_sent' THEN 3
|
||||
WHEN 'in_comms' THEN 2
|
||||
WHEN 'in_communication' THEN 2
|
||||
WHEN 'details_sent' THEN 1
|
||||
ELSE 0
|
||||
END`;
|
||||
|
||||
@@ -50,14 +50,18 @@ export async function fetchPipelineData(
|
||||
portId: string,
|
||||
_params: Record<string, unknown>,
|
||||
): Promise<PipelineData> {
|
||||
// Count interests per pipeline stage (non-archived)
|
||||
// Count interests per pipeline stage (non-archived).
|
||||
// The reporting audit caught the missing .groupBy() — without it,
|
||||
// postgres rejects the SELECT or collapses every interest into a
|
||||
// single ELSE-stage row. groupBy fixes the per-stage breakdown.
|
||||
const stageCounts = await db
|
||||
.select({
|
||||
stage: interests.pipelineStage,
|
||||
count: count(),
|
||||
})
|
||||
.from(interests)
|
||||
.where(and(eq(interests.portId, portId), isNull(interests.archivedAt)));
|
||||
.where(and(eq(interests.portId, portId), isNull(interests.archivedAt)))
|
||||
.groupBy(interests.pipelineStage);
|
||||
|
||||
const stageCountMap: Record<string, number> = {};
|
||||
for (const row of stageCounts) {
|
||||
@@ -122,7 +126,11 @@ export async function fetchRevenueData(
|
||||
stageRevenueMap[row.stage] = row.revenue ? String(row.revenue) : '0';
|
||||
}
|
||||
|
||||
// Total revenue from completed interests (primary-berth link only).
|
||||
// Total revenue from WON interests only. Reporting audit caught the
|
||||
// gap: setInterestOutcome forces pipelineStage='completed' for lost
|
||||
// AND cancelled outcomes too, so filtering by stage alone counted
|
||||
// those toward "TOTAL COMPLETED REVENUE". The outcome='won' filter is
|
||||
// the canonical money-changed-hands signal.
|
||||
const completedRevenue = await db
|
||||
.select({ total: sum(berths.price) })
|
||||
.from(interests)
|
||||
@@ -135,6 +143,7 @@ export async function fetchRevenueData(
|
||||
and(
|
||||
eq(interests.portId, portId),
|
||||
eq(interests.pipelineStage, 'completed'),
|
||||
eq(interests.outcome, 'won'),
|
||||
isNull(interests.archivedAt),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -1908,11 +1908,38 @@ export async function search(
|
||||
// Merge direct matches with expansion rows; direct rows always win
|
||||
// ties and sort first. Each bucket caps at `limit * 2` so reps still
|
||||
// see the full direct-match set plus a healthy expansion tail.
|
||||
const mergedClients = mergeWithExpansion(clients, expanded.clients, limit * 2);
|
||||
const mergedInterests = mergeWithExpansion(interests, expanded.interests, limit * 2);
|
||||
const mergedYachts = mergeWithExpansion(yachts, expanded.yachts, limit * 2);
|
||||
const mergedCompanies = mergeWithExpansion(companies, expanded.companies, limit * 2);
|
||||
const mergedBerths = mergeWithExpansion(berths, expanded.berths, limit * 2);
|
||||
//
|
||||
// SECURITY (search-auditor H1): expandGraph runs unconditionally,
|
||||
// but its results MUST be re-gated by the destination bucket's view
|
||||
// permission. A user with berths.view but no interests.view searching
|
||||
// "A12" was previously getting interest rows (client name + stage)
|
||||
// surfaced via expansion. Gate each merge call so the expansion
|
||||
// contributes empty rows for any bucket the caller can't see.
|
||||
const mergedClients = mergeWithExpansion(
|
||||
clients,
|
||||
can(opts, 'clients.view') ? expanded.clients : [],
|
||||
limit * 2,
|
||||
);
|
||||
const mergedInterests = mergeWithExpansion(
|
||||
interests,
|
||||
can(opts, 'interests.view') ? expanded.interests : [],
|
||||
limit * 2,
|
||||
);
|
||||
const mergedYachts = mergeWithExpansion(
|
||||
yachts,
|
||||
can(opts, 'yachts.view') ? expanded.yachts : [],
|
||||
limit * 2,
|
||||
);
|
||||
const mergedCompanies = mergeWithExpansion(
|
||||
companies,
|
||||
can(opts, 'companies.view') ? expanded.companies : [],
|
||||
limit * 2,
|
||||
);
|
||||
const mergedBerths = mergeWithExpansion(
|
||||
berths,
|
||||
can(opts, 'berths.view') ? expanded.berths : [],
|
||||
limit * 2,
|
||||
);
|
||||
|
||||
const result: SearchResults = {
|
||||
clients: apply(mergedClients),
|
||||
|
||||
Reference in New Issue
Block a user