Skip to content

Commit 5082608

Browse files
tofikwesttofikwest
committed
Merge remote-tracking branch 'origin/main' into tofik/sq-extension-review-fixes
# Conflicts: # apps/api/src/auth/auth.server.ts # bun.lock
2 parents 16487c1 + 1952330 commit 5082608

1,504 files changed

Lines changed: 175263 additions & 33899 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/agents/security-reviewer.md‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
name: security-reviewer
3+
description: Reviews code for the most common high-risk security vulnerabilities (broken access control, tenant isolation, injection, secrets, SSRF, auth/session, unsafe file handling) in the NestJS + Next.js + Prisma + better-auth monorepo
4+
tools: Read, Grep, Glob, Bash
5+
---
6+
7+
You are a senior application security engineer reviewing a **multi-tenant compliance SaaS**: NestJS API (`apps/api`), Next.js app (`apps/app`), Prisma (`packages/db`), better-auth (auth lives in the API). Review the files/diff you are given against the vulnerability classes below.
8+
9+
## How to review
10+
11+
- **Trace the real code — do not speculate.** Follow the request from guard → controller → service → DB. Read `apps/api/src/auth/permission.guard.ts` and `hybrid-auth.guard.ts` when authz is in question.
12+
- For every finding report: **severity** (`P1` critical / `P2` high / `P3` medium), `file:line`, the **concrete exploit + impact**, and the **fix**. Rank most-severe first.
13+
- If a class is clean, say so in one line with the evidence. Prefer precision over volume — no speculative or style noise.
14+
- **Don't re-flag what CI already owns**: CodeQL (SAST), Dependabot + `sbom.yml` (dependency CVEs / supply chain). Focus on logic/design issues those miss.
15+
16+
## Vulnerability classes (highest-risk first)
17+
18+
### 1. Broken access control (the #1 risk here)
19+
- Every mutation endpoint (POST/PATCH/PUT/DELETE) has `@UseGuards(HybridAuthGuard, PermissionGuard)` + `@RequirePermission('resource','action')`. GETs need `('resource','read')`. `@Public()` only on webhooks.
20+
- **API-key & service-token scope enforcement**: authz is by `request.apiKeyScopes` (API key) / the service's own `permissions` (service token) — never by the acting user's roles. Flag anything that authorizes off a *resolved/attributed* user.
21+
- **Authz ≠ attribution**: the result of `ActingUserResolver.resolve()` (`acting.userId`/`memberId`) may be used ONLY for audit rows / `createdBy` / assignment — never to make a permission decision.
22+
- **IDOR**: any lookup/update/delete by an id from the client (`@Param`, body) must also be scoped so the caller can only reach their own org's row.
23+
24+
### 2. Multi-tenant isolation
25+
- **Every** Prisma query is scoped by `organizationId`, and that org id comes from the auth context (`@OrganizationId()` / `request.organizationId`), NEVER from a client-supplied body/param/header (except the validated `x-organization-id` service-token path).
26+
- Cross-org reference: creating/linking a row must verify the referenced id belongs to the same org.
27+
28+
### 3. Injection
29+
- Prisma raw APIs: `$queryRawUnsafe` / `$executeRawUnsafe` / string-interpolated `$queryRaw` = SQL injection. Require parameterized tagged templates or `Prisma.sql`.
30+
- Command injection (`child_process` exec/spawn with interpolated input), path traversal in file paths (`../`), and unsafe `req.body` handling.
31+
32+
### 4. XSS / output handling (Next.js)
33+
- `dangerouslySetInnerHTML`, unsanitized rich text (TipTap/HTML content) rendered without sanitization, `href`/redirect built from user input (`javascript:` URLs), unvalidated open redirects.
34+
35+
### 5. Secrets & sensitive-data exposure
36+
- Hardcoded secrets/keys/tokens; secrets or full tokens written to logs; `.env` committed; secrets returned in API responses or echoed to the client.
37+
- **PII / secret in the audit or activity data JSON.** (This repo has a history of keys leaking via git notes — flag any new secret material in tracked files.)
38+
39+
### 6. Authentication & session
40+
- All auth goes through the API (no local better-auth instance in app/portal). Session checks proxy `/api/auth/get-session`. No JWTs. Raw `fetch()` to the API includes `credentials: 'include'`.
41+
- Trust boundaries: service-token `x-user-id` and API-key attribution must resolve to an **active** member (`deactivated:false`, `isActive:true`) of the bound org.
42+
43+
### 7. SSRF & outbound requests
44+
- Server-side `fetch`/HTTP to a **user-controlled URL** (vendor website research, integrations, cloud connectors, webhooks) — validate scheme/host, block internal/link-local ranges (169.254.169.254, 127.0.0.0/8, 10/172.16/192.168), don't follow redirects into internal space.
45+
46+
### 8. Unsafe file upload / storage
47+
- S3 presigned URLs scoped to the org's prefix + constrained content-type/size; uploaded content-type validated; no path traversal in object keys; buckets not public (OAC/BPA).
48+
49+
### 9. Mass assignment
50+
- Spreading unvalidated `...dto` / `...req.body` directly into `db.*.create/update` lets a caller set fields they shouldn't (status, ownerId, organizationId, flags). Require an explicit field allowlist for anything security-relevant.
51+
52+
### 10. DoS / resource exhaustion
53+
- Unbounded queries (no pagination/limit), expensive synchronous work on the request path (offload to Trigger.dev), unbounded loops over caller-supplied arrays. (This repo took prod down via an in-process bulk export.)
54+
55+
### 11. Error handling / info disclosure
56+
- Internal errors, stack traces, or DB/driver messages returned to the client. Use NestJS exceptions with clean messages.
57+
58+
### 12. Audit trail / non-repudiation
59+
- Security-relevant mutations are audit-logged, attributed to the acting user, and non-session callers carry a provenance marker (`ActingUserResolver` `callerLabel` → `via API key "…"`), so an owner-fallback attribution isn't mistaken for a session action.
60+
61+
## Output
62+
A ranked list of findings (severity, `file:line`, exploit/impact, fix), then a one-line "clean" note for each class with no findings. End with the single highest-priority action.

‎.claude/skills/check-results-service/SKILL.md‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
---
2+
name: check-results-service
3+
description: How to reuse ANY integration check's results in a feature via the universal CheckResultsService (apps/api integration-platform). Use whenever a feature needs data produced by an integration check — "show 2FA status on People", "surface AWS S3 findings in X", "reuse a check's results", "per-user/per-resource results from a connected integration", "which integrations feed task T". Read this BEFORE writing your own IntegrationCheckResult / CheckRunRepository query — don't hand-roll it.
4+
---
5+
6+
# CheckResultsService — reuse any integration check's results
7+
8+
## The one idea
9+
10+
Integration checks (2FA, AWS S3 encryption, device posture, …) all write per-resource
11+
results to one table (`IntegrationCheckResult`). **`CheckResultsService` is the single,
12+
universal, read-only way to get those results into any feature.** It is deliberately
13+
**feature-agnostic**: it fetches results and hands them back in a stable envelope, and it
14+
does **not** know or care what the data means.
15+
16+
> **Service = fetch the results (generic). Feature = interpret + map + present.**
17+
18+
If you're about to query `IntegrationCheckResult` or `CheckRunRepository` directly from a
19+
new feature — stop. Use this service instead.
20+
21+
- Service: `apps/api/src/integration-platform/services/check-results.service.ts`
22+
- Exported from `IntegrationPlatformModule` — inject it into any feature module.
23+
- Reference consumer (copy this): the 2FA feature — `two-factor-source.controller.ts`.
24+
25+
## When to use it
26+
27+
Use it whenever a feature needs the output of an integration check:
28+
- "Show each employee's 2FA status on the People tab" (the current consumer)
29+
- "Surface which S3 buckets failed encryption inside feature X"
30+
- "Show device compliance per user from the MDM check"
31+
- "List which connected integrations can supply data for task T"
32+
33+
## The API (3 methods)
34+
35+
```ts
36+
// 1. Discover sources: which connected integrations can feed a task, + connection state.
37+
listSourcesBoundToTask(organizationId, taskTemplateId): Promise<CheckSourceInfo[]>
38+
39+
// 2. The primitive: full results of a check's latest REAL run for ONE connection.
40+
getLatestResultsByCheck({ organizationId, connectionId, checkId, resourceType? }): Promise<CheckResultRow[]>
41+
42+
// 3. Convenience: results for a task-bound check from a chosen source (provider slug).
43+
// Resolves task -> check and slug -> connection for you.
44+
getLatestResultsForTask({ organizationId, taskTemplateId, sourceSlug, resourceType? }): Promise<CheckResultRow[]>
45+
```
46+
47+
Notes:
48+
- "Latest REAL run" = newest run that isn't `inconclusive`/held and whose connection isn't
49+
disconnected. Held runs (our-side self-heal) never leak to a feature.
50+
- No row cap — you get the full result set (unlike the 30-row task-history display).
51+
- `resourceType` filters rows (e.g. `'user'`, `'bucket'`). Omit to get all.
52+
- Empty array = "no data" (source not bound/connected, or never really ran). Never throws
53+
for "no results".
54+
55+
## Person-scoped results: the shape contract
56+
57+
Checks about PEOPLE (employee access, 2FA/MFA, training) follow a standard emission shape —
58+
this section is the canonical definition of it:
59+
60+
- **One row per person** — never a single aggregate row with a roster buried in `evidence`.
61+
- **`resourceType: 'user'`** (exactly this string).
62+
- **`resourceId` = the person's email, lowercased + trimmed.** Fallback `username || id`
63+
only when the provider genuinely exposes no email (such rows won't join to members —
64+
acceptable, still visible in evidence views).
65+
- **`evidence`** carries what the provider knows: `email`, `name`, `role`, `isAdmin`,
66+
`status`, `lastLogin`, plus a `checkedAt` timestamp.
67+
- **Access/inventory rows always emit as pass** (having access is information, not a
68+
violation); compliance-gate checks (2FA, training) pass/fail per person; error paths
69+
(bad creds, missing scopes) stay org-level rows.
70+
71+
So a feature joining check results to org members does exactly this — no parsing, no AI:
72+
73+
```ts
74+
const rows = await checkResults.getLatestResultsForTask({
75+
organizationId, taskTemplateId, sourceSlug, resourceType: 'user',
76+
});
77+
const forMember = rows.filter((r) => r.resourceId === member.email.toLowerCase());
78+
```
79+
80+
If a source returns zero `'user'` rows, its check hasn't been normalized to the standard
81+
yet (or genuinely has no per-person data) — render that as "no per-person data from this
82+
source", and fix the CHECK to emit the shape above. Never work around it by parsing
83+
aggregate evidence in the feature.
84+
85+
## The envelope you get back
86+
87+
```ts
88+
interface CheckResultRow {
89+
resourceId: string; // provider-native id: email (2FA), bucket ARN (S3), repo, …
90+
resourceType: string; // 'user' | 'bucket' | …
91+
passed: boolean; // did this resource pass the check
92+
title: string;
93+
description: string | null;
94+
evidence: Prisma.JsonValue; // ← check-SPECIFIC payload. The service does NOT interpret it.
95+
collectedAt: Date;
96+
runId: string;
97+
connectionId: string;
98+
}
99+
```
100+
101+
**The `evidence` rule (important):** the envelope is universal; the check-specific data lives
102+
in `evidence` as raw JSON. The service never types or interprets it. **Your feature validates
103+
`evidence` at its own edge with a zod schema** and reads only the fields it understands:
104+
105+
```ts
106+
const TwoFaEvidence = z.object({ isEnrolledIn2Sv: z.boolean() });
107+
const parsed = TwoFaEvidence.safeParse(row.evidence);
108+
// use parsed.data.isEnrolledIn2Sv — never `as any`
109+
```
110+
111+
For a simple pass/fail feature you often don't even need `evidence` — just use `row.passed`
112+
and `row.resourceId` (that's all 2FA needs).
113+
114+
## How to add a NEW consumer (step by step)
115+
116+
Say a feature wants "which AWS S3 buckets failed encryption":
117+
118+
1. Inject the service: add `CheckResultsService` to your feature's constructor (the module
119+
already exports it; import `IntegrationPlatformModule` if your module doesn't already).
120+
2. Get results with the primitive (S3 usually spans many AWS connections, so you may loop
121+
connections from `listSourcesBoundToTask` or your own connection lookup):
122+
```ts
123+
const rows = await this.checkResults.getLatestResultsByCheck({
124+
organizationId,
125+
connectionId,
126+
checkId: 'aws-s3-encryption', // the check's manifest id
127+
resourceType: 'bucket',
128+
});
129+
```
130+
…or, if your feature lets the user PICK a source for a task (like 2FA does), use
131+
`getLatestResultsForTask` with the task template id and the chosen `sourceSlug`.
132+
3. Interpret in YOUR feature: map `resourceId`/`passed`, validate `evidence` with zod, shape
133+
the response your UI needs. **Do not** add feature logic to the service.
134+
4. Test with the service mocked (see `two-factor-source.controller.spec.ts`).
135+
136+
## Worked example — the 2FA feature (reference implementation)
137+
138+
The People-tab 2FA column is consumer #1. Study it end to end:
139+
140+
- `two-factor-source.controller.ts`
141+
- `available-2fa-sources` → `listSourcesBoundToTask(org, TASK_TEMPLATES.twoFactorAuth)`
142+
- `two-factor-statuses` → `getLatestResultsForTask({ org, taskTemplateId: twoFactorAuth,
143+
sourceSlug: org.twoFactorSource, resourceType: 'user' })`, then maps `resourceId`→email
144+
(lowercased) and `passed`→`enabled`/`missing`. Emails with no row are resolved to
145+
"Not provided" on the client, never a false "missing".
146+
- The controller owns the 2FA-specific bits (the `Organization.twoFactorSource` column, the
147+
enabled/missing interpretation). The generic fetch is all in the service.
148+
149+
## Rules / do & don't
150+
151+
- ✅ DO put every "read a check's results" path through this service.
152+
- ✅ DO validate `evidence` with zod in your feature. No `as any`.
153+
- ✅ DO treat an empty array as "no data / not provided" — never a failure.
154+
- ❌ DON'T query `IntegrationCheckResult` / `CheckRunRepository` directly from a feature.
155+
- ❌ DON'T add feature-specific interpretation (enabled/missing, domain mapping) to the
156+
service — it must stay universal and read-only.
157+
- ❌ DON'T assume a source produces per-`user` (or email-keyed) rows — that's per-check. If a
158+
chosen source's check emits a different `resourceType` or a non-mappable `resourceId`, your
159+
feature should degrade gracefully (e.g. "Not provided"), not crash.
160+
161+
## Source selection is NOT part of this service
162+
163+
Which integration an org uses for a purpose (e.g. `Organization.twoFactorSource`) is
164+
feature-owned config, mirroring `employeeSyncProvider` / `deviceSyncProvider`. Add a column
165+
per feature. (If a 3rd/4th feature needs it, consider generalizing selection then — not
166+
before.)

‎.claude/skills/production-readiness/SKILL.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ disable-model-invocation: true
66

77
Run a comprehensive production readiness check on $ARGUMENTS.
88

9-
Use parallel subagents to run all four audits simultaneously:
9+
Use parallel subagents to run all five audits simultaneously:
1010
1. audit-rbac on $ARGUMENTS
1111
2. audit-hooks on $ARGUMENTS
1212
3. audit-design-system on $ARGUMENTS
1313
4. audit-tests on $ARGUMENTS
14+
5. security-review on $ARGUMENTS
1415

1516
Then run full monorepo verification:
1617
```bash

‎.claude/skills/responsive-ui/SKILL.md‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
---
2+
name: responsive-ui
3+
description: MANDATORY for any UI work in apps/app, apps/portal, or packages/design-system — every component, page, or layout change must work on mobile (~375px), tablet (~768px), desktop (~1280px), and large desktop (~1920px) BY DEFAULT, without being asked. Read this before writing or editing any JSX/TSX that renders visible UI. Triggers on: new component, page, layout, table, toolbar, form, modal/sheet, dashboard, "build UI", "add a column", "add a filter", any styling change.
4+
---
5+
6+
# Responsive UI — default, not a feature request
7+
8+
## The rule
9+
10+
**Every UI change ships working at all four device classes. Nobody has to ask.**
11+
12+
| Device | Test width | Tailwind context |
13+
|---|---|---|
14+
| Mobile | 375px | base (no prefix) |
15+
| Tablet | 768px | `md:` |
16+
| Desktop | 1280px | `xl:` |
17+
| Large desktop | 1920px | beyond `2xl:` |
18+
19+
Tailwind is **mobile-first**: unprefixed classes are the mobile layout; prefixes add
20+
behavior at wider screens (`sm:` 640, `md:` 768, `lg:` 1024, `xl:` 1280, `2xl:` 1536).
21+
Write the mobile layout first, then widen — not the other way around.
22+
23+
A change is NOT done until you have reasoned through (or better, viewed via the
24+
`webapp-testing` skill / browser dev tools) what it looks like at 375, 768, 1280,
25+
and 1920. If you only checked one width, you checked none.
26+
27+
## Works WITH the design system — precedence rules
28+
29+
This skill layers on top of the `ui` skill; **where they touch, the design-system
30+
rules win**. Responsiveness is achieved THROUGH the DS, never around it:
31+
32+
- **Never put responsive classes (or any `className`) on DS components**
33+
(`Text`, `Stack`, `HStack`, `Badge`, `Button`, …) — they don't accept it. Put
34+
breakpoint utilities on a wrapper `<div>` (the `ui` skill's "Layout with Wrapper
35+
Divs" pattern):
36+
```tsx
37+
// ✅ breakpoints on the wrapper, DS component untouched
38+
<div className="hidden sm:block"><Badge variant="outline">Active</Badge></div>
39+
// ❌ className on a DS component
40+
<Badge className="hidden sm:block">Active</Badge>
41+
```
42+
- **Reach for DS layout primitives first** (`PageLayout`, `Stack`, `HStack`,
43+
`Section`) — they carry responsive spacing already. Raw responsive divs are for
44+
what the primitives don't cover, not a replacement for them.
45+
- **Arbitrary pixel values stay an anti-pattern** (`w-[847px]` ❌ — per the `ui`
46+
skill). Prefer the standard Tailwind scale (`max-w-sm`, `w-48`, `max-w-xs`). A
47+
fixed control width like `w-[200px]` is acceptable ONLY when it matches an
48+
established pattern already used on that surface, and it must still carry a
49+
responsive strategy (see below).
50+
51+
## Non-negotiables
52+
53+
1. **The page body never scrolls horizontally.** Wide content (tables, code, diagrams)
54+
scrolls inside its own container, never the page.
55+
2. **No fixed width without a responsive strategy.** A fixed-width control is only
56+
acceptable if the element hides, shrinks, or wraps on smaller screens
57+
(`hidden sm:block`, `w-full md:max-w-xs`, or a wrapping parent) — with the
58+
breakpoint classes on a wrapper div, never on a DS component.
59+
3. **Touch targets** on mobile: interactive elements ≥40px tall; don't rely on hover
60+
for anything essential (no hover on touch devices).
61+
4. **Test with real content lengths** — long names, emails, 33-item counts. Use
62+
`truncate` / `min-w-0` on flex children that hold text.
63+
5. **Large desktops:** content must not stretch into unreadable full-width lines —
64+
cap with `max-w-*` containers where the layout doesn't already.
65+
66+
## Repo patterns (use these, don't invent)
67+
68+
- **Tables**: the design-system `Table` already wraps rows in `overflow-x-auto` —
69+
wide tables scroll horizontally inside themselves on mobile. That is the accepted
70+
pattern for dense admin tables. Do NOT try to reflow table columns per breakpoint;
71+
do keep cells reasonable (`min-w-* max-w-*` on cell content is fine).
72+
- **Toolbars / filter rows**: primary control (search) is `w-full md:max-w-[300px]`;
73+
secondary controls (filters, source selectors) collapse on phones with
74+
`hidden sm:block`. Example: the People tab toolbar in
75+
`apps/app/.../people/all/components/TeamMembersClient.tsx`.
76+
- **Page shells**: use design-system `PageLayout` / `Stack` / `HStack` — they carry
77+
the responsive spacing; don't rebuild them with raw divs.
78+
- **Grids**: `grid-cols-1 md:grid-cols-2 xl:grid-cols-3` style progressions — never a
79+
fixed column count for all widths.
80+
- **Sheets/Modals**: design-system `Sheet`/`Dialog` are responsive already; don't fix
81+
their widths with arbitrary values.
82+
- **Images/logos**: constrain (`max-w-full`, explicit small sizes); never let an
83+
image force layout width.
84+
85+
## Checklist before "done" (run mentally or in the browser)
86+
87+
- [ ] 375px — nothing overflows the viewport; controls usable or intentionally hidden; text truncates instead of breaking layout
88+
- [ ] 768px — secondary controls visible; layout uses available width sensibly
89+
- [ ] 1280px — the intended "design" width; matches mockups
90+
- [ ] 1920px — no absurdly stretched content; whitespace balanced
91+
- [ ] Long content (names, emails, high counts) doesn't break any of the above
92+
- [ ] Tests: if a component hides/reflows per breakpoint in a way that matters, its test asserts the class contract (e.g. `hidden sm:block`)
93+
94+
## When you can skip a width
95+
96+
Admin-only dense screens (e.g. internal tooling) may treat mobile as "usable via
97+
horizontal scroll" rather than fully reflowed — but that must be a conscious choice
98+
following the existing pattern of that page, never an accident of not checking.

0 commit comments

Comments
 (0)