-
Notifications
You must be signed in to change notification settings - Fork 365
Expand file tree
/
Copy pathCONTRACTS
More file actions
514 lines (408 loc) · 20.1 KB
/
Copy pathCONTRACTS
File metadata and controls
514 lines (408 loc) · 20.1 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
@cc [label:coding] reuse-existing-patterns
Favor re-using existing patterns (with broad refactors if necessary) over introducing new ones
sporadically.
Reviewer: If you detect a pattern that is not consistent with an existing approach in the codebase,
require the author to match the existing pattern (and possibly refactor everything in a subsequent
PR).
@cc [label:coding] prefer-simple-solutions
Favor simple and easy to understand approaches vs. overly optimized but complex ones.
Reviewer: If you detect an overly optimized or complex solution that can be simplified (at the cost
of a bit of performance loss or extra code), ask the author to consider the simpler approach.
@cc [label:coding] prefer-types-over-typescript-enums
We do not use TypeScript enums, we use types instead, e.g.: `type Color = "red" | "blue";`.
@cc [label:coding] no-unsafe-type-assertions
The non-type-safe uses of `as` are prohibited in the codebase. Use type guards or other type-safe
methods instead. There are few exceptions where `as` is type-safe to use (e.g., `as const`) and
therefore acceptable.
@cc [label:coding] no-parameter-mutation
Never mutate arrays or objects passed as parameters to functions. Create and return new instances
instead. This includes avoiding methods like `splice` that mutate arrays in place.
Reviewer: If you detect parameter mutation in the code (including array methods like `splice`),
request the author to refactor the code to create and return new instances instead.
Example:
```
// BAD
function addItem(items: string[], newItem: string) {
items.push(newItem);
return items;
}
// GOOD
function addItem(items: string[], newItem: string) {
return [...items, newItem];
}
```
@cc [label:coding] exhaustive-union-branching
When branching on a discriminated union or string union, prefer an exhaustive `switch` with
`assertNever` (from `@app/types/shared/utils/assert_never`) over chains of `if`/`else` or nested
ternaries. This keeps the code readable and ensures TypeScript enforces exhaustiveness when cases
are added.
Two variants are available:
- **`assertNever`**: Throws at runtime. Use when missing a case is a bug (server-side code,
internal client logic, client-created data).
- **`assertNeverAndIgnore`**: Does NOT throw at runtime. Use in client-side code that processes
API data (event streams, API responses, connector types, etc.) where the server may add new
enum values before the client is updated. The unknown value is silently ignored instead of
crashing the app.
Both provide the same compile-time exhaustiveness checking. Using the wrong variant is a bug:
using `assertNever` on API data can crash the app when the server adds a new value, and using
`assertNeverAndIgnore` on internal logic can silently swallow programming errors.
Example:
```
import { assertNever, assertNeverAndIgnore } from "@app/types/shared/utils/assert_never";
// Internal logic: use assertNever (crash on missing case)
type Status = "approved" | "rejected" | "expired";
function titleForStatus(status: Status): string {
switch (status) {
case "approved":
return "Approved";
case "rejected":
return "Rejected";
case "expired":
return "Expired";
default:
return assertNever(status);
}
}
// Processing API data: use assertNeverAndIgnore (gracefully ignore unknown values)
function handleStreamEvent(event: AgentMessageEvent): State {
switch (event.type) {
case "generation_tokens":
return { ...state, content: state.content + event.text };
case "agent_error":
return { ...state, status: "error" };
default:
assertNeverAndIgnore(event);
return state;
}
}
```
@cc [owner:philipperolet,label:coding] avoid-quadratic-loops
Loops with quadratic O(n²) or worse, cubic O(n³) complexity can severely hurt performance as data
sizes grow. Common quadratic patterns include nested loops over related datasets, repeated searches
within loops, and chained array operations that each iterate over the data.
Always prefer linear O(n) or logarithmic O(n log n) solutions using data structures like Map, Set,
or sorted arrays. When quadratic complexity is unavoidable, ensure array sizes are small (< 100
elements) and comment the code appropriately with expected array sizes and execution time. For
larger datasets or longer operations, implement async processing or move to separate workflows.
Example:
```
// BAD - O(n²) nested loop
function findDuplicates(items: Item[], otherItems: Item[]) {
const duplicates = [];
for (const item of items) {
for (const other of otherItems) {
if (item.id === other.id) {
duplicates.push(item);
}
}
}
return duplicates;
}
// GOOD - O(n) using Set lookup
function findDuplicates(items: Item[], otherItems: Item[]) {
const otherIds = new Set(otherItems.map(item => item.id));
return items.filter(item => otherIds.has(item.id));
}
// BAD - O(n²) repeated find operation in map
const enrichedAgents = agents.map(agent => ({
...agent,
isFavorite: members.find(m => m.favoriteAgentId === agent.sId) !== undefined
}));
// GOOD - O(n) using Set for constant-time lookup
const favoriteAgentIds = new Set(members.map(m => m.favoriteAgentId));
const enrichedAgents = agents.map(agent => ({
...agent,
isFavorite: favoriteAgentIds.has(agent.sId)
}));
// BAD - O(n²) nested loops without bounds checking
for (const workspace of workspaces) {
for (const member of workspace.members) {
processWorkspaceMember(workspace, member);
}
}
// GOOD - Comment when quadratic is acceptable due to small array sizes
function validatePermissions(userRoles: string[], requiredPermissions: string[]) {
// O(n²) acceptable: both arrays guaranteed to be small (< 20 elements each).
return requiredPermissions.every(permission =>
userRoles.some(role => hasPermission(role, permission))
);
}
```
When quadratic complexity cannot be avoided:
- Comment the expected maximum array sizes and execution time
- Add runtime assertions if sizes could exceed safe limits
- Consider moving to background processing for larger datasets
- Implement proper async handling with progress indicators for long operations
@cc [label:coding] use-application-logger
Direct calls to `console.log`, `console.error`, `console.warn`, `console.info`, or similar console
methods are prohibited in the codebase. Always use the application logger for all logging,
debugging, and error reporting purposes. This ensures consistent log formatting, proper log
routing, and easier log management across environments.
Example:
```
// BAD
console.log("User created", user);
console.error("Failed to fetch data", error);
// GOOD
logger.info({ user }, "User created");
logger.error({ err: error }, "Failed to fetch data");
```
@cc [label:coding] money-and-time-unit-suffixes
Variables representing monetary amounts or time durations must include a unit suffix in their name.
This prevents conversion errors (e.g., cents vs. dollars, milliseconds vs. seconds) such as the one
that caused [this incident](https://dust4ai.slack.com/archives/C05B529FHV1/p1764835038528229).
Common suffixes:
- Money: `Cents`, `Dollars` (e.g., `priceCents`, `amountDollars`)
- Time: `Ms`, `Seconds`, `Minutes`, `Hours` (e.g., `timeoutMs`, `durationSeconds`)
This rule does not apply to common Sequelize date/timestamp fields that follow the framework
convention, such as `createdAt` and `updatedAt`.
Reviewer: If you detect a variable representing money or time without a unit suffix, require the
author to rename it with the appropriate suffix.
Example:
```
// BAD
const price = 1999;
const timeout = 5000;
const delay = 30;
// GOOD
const priceCents = 1999;
const timeoutMs = 5000;
const delaySeconds = 30;
```
@cc [label:coding] prefer-static-imports
In production TypeScript/TSX code, prefer static imports at the top of the file.
Use dynamic `import()` only when strictly necessary (e.g., runtime gating between Node/Edge,
optional dependencies, or excluding client-only code from server bundles).
This rule does not apply to CommonJS config files (e.g., `next.config.js`, `tailwind.config.js`)
or to Vitest patterns such as `vi.mock(import("..."), ...)`.
If a dynamic import is strictly necessary, it must:
- Use a string literal module specifier (no computed paths).
- Be accompanied by a short comment explaining why a static import is not acceptable.
@cc [label:coding] read-environment-through-config
Never access environment variables directly via `process.env`. Instead, use the `@app/lib/api/config`
module which provides type-safe access to environment variables.
GitHub workflows and action scripts under `.github/` are out of scope and may use `process.env`.
Example:
```
// BAD
const apiUrl = process.env.API_URL;
const isProduction = process.env.NODE_ENV === "production";
// GOOD
import config from "@app/lib/api/config";
const apiUrl = config.getApiUrl();
const isProduction = config.getNodeEnv() === "production";
```
@cc [label:coding] no-css-important
Do not use `!important` in stylesheets, CSS modules, Tailwind `@apply` blocks, CSS-in-JS, or
template literals that produce CSS. This also covers Tailwind's `!` important-prefix utilities
(e.g. `focus:!ring-0`, `!mt-0`) — they compile to `!important` and have the same downsides.
`!important` breaks the cascade, makes specificity issues harder to debug, and tends to spread
once introduced — every override forces the next one.
Fix the underlying specificity issue instead: increase selector specificity, reorder rules, scope
via a parent class, drop conflicting utilities, or restructure the component.
Narrow exceptions are acceptable when:
- Overriding styles from a third-party library that we do not control.
- Dev-only tooling that injects inspection/debug styles at runtime.
In those cases, add a brief comment on the same line or directly above explaining why
`!important` is required.
Reviewer: If you detect `!important` (including Tailwind's `!` prefix) without a justifying
comment, ask the author to remove it and address the underlying specificity issue.
Example:
```
/* BAD */
.button {
color: red !important;
}
/* GOOD */
.toolbar .button {
color: red;
}
/* GOOD — third-party library override */
.allotment-pane {
/* allotment computes inline styles; only !important wins. */
cursor: default !important;
}
```
```tsx
// BAD — Tailwind important prefix
<div className="focus:!ring-0 !mt-0" />
// GOOD — drop the conflicting utility, or scope via a parent
<div className="focus:ring-transparent mt-0" />
```
@cc [label:coding] use-zod-for-runtime-validation
We are migrating off `io-ts` in favor of `zod`. All new schemas (request bodies, query
parameters, form validation, runtime parsing of external data) must be written with `zod`. Do
not introduce new `io-ts` codecs, and do not import `io-ts`, `io-ts-types`, `io-ts-reporters`,
or `fp-ts/lib/Either` in new code.
Existing `io-ts` codecs may remain — they will be migrated incrementally. When modifying a file
that already uses `io-ts`, prefer migrating its codecs to `zod` if the change is local; if the
codec is shared across many consumers, leave the migration to a dedicated PR.
For error formatting, use `fromError` from `zod-validation-error` to produce a readable message
from a `ZodError`.
Example:
```
// BAD — new code using io-ts
import * as t from "io-ts";
import { isLeft } from "fp-ts/lib/Either";
import * as reporter from "io-ts-reporters";
const BodySchema = t.type({
name: t.string,
count: t.number,
});
const validation = BodySchema.decode(req.body);
if (isLeft(validation)) {
const message = reporter.formatValidationErrors(validation.left);
return apiError(req, res, { status_code: 400, api_error: { type: "invalid_request_error", message } });
}
const { name, count } = validation.right;
// GOOD — new code using zod
import { z } from "zod";
import { fromError } from "zod-validation-error";
const BodySchema = z.object({
name: z.string(),
count: z.number(),
});
const validation = BodySchema.safeParse(req.body);
if (!validation.success) {
return apiError(req, res, {
status_code: 400,
api_error: {
type: "invalid_request_error",
message: fromError(validation.error).toString(),
},
});
}
const { name, count } = validation.data;
```
Reviewer: If you see new `io-ts` imports or codecs introduced by a PR, require the author to
rewrite them in `zod`.
@cc [owner:flvndvd,label:coding] batch-database-queries
Avoid database queries or database-only helpers inside loops, which would resolve in an unbounded
number of SQL queries that will scale with the data (N+1 pattern).
Also avoid them in `Promise.all` and `concurrentExecutor` if possible, they only make the queries
run in parallel; they do not fix the N+1. Running too many SQL queries in parallel can quickly put
pressure on our connection pool and even exhaust it. `concurrentExecutor` is a good fit for
interacting with external services that are beyond our control, not for database queries.
Prefer batching instead: fetch the related rows with one scoped query.
Example:
```
// BAD: one user query per membership + unbounded Promise.all that take several exhaust the connection pool.
const users = await Promise.all(
memberships.map((membership) =>
UserResource.fetchByModelId(membership.userId)
)
);
// GOOD: one query, then reconstruct by id.
const users = await UserResource.fetchByModelIds(
memberships.map((membership) => membership.userId)
);
```
Reviewer: If you detect a DB-backed helper or query inside a loop, ask the author to batch it.
@cc [label:security] sensitive-data-in-bodies-or-headers
No sensitive data should be sent to our servers through URL or query string parameters. HTTP body or
headers only are acceptable for sensitive data.
@cc [owner:flvndvd,label:security] string-ids-in-api-interfaces
Never expose or accept ModelId in URLs, API endpoints or POST/PATCH payloads. Use string identifiers
(sId) instead. This applies to all routes, including GET, POST, PATCH, and DELETE methods. ModelIds
should be strictly internal and never exposed to the client.
Example:
```
// BAD
/api/w/[wId]/resource/[modelId]
// GOOD
/api/w/[wId]/resource/[sId]
```
@cc [label:security] sandbox-root-command-safety
Any runtime command that runs in an already-built sandbox as `root` must invoke executables through
absolute paths. This applies to `sandbox.execRoot(...)`, provider-level E2B command execution as
root, wake/resume hooks, egress setup, log readers, and root-invoked helper scripts.
Image build DSL commands in `front/lib/api/sandbox/image/registry.ts` are out of scope for this
runtime PATH-hijack rule because they run while building the image, before untrusted sandbox
workloads can plant files in the sandbox filesystem. Prefer absolute paths there when practical,
but review them as image build reproducibility/hardening issues rather than runtime workload
escape issues.
Runtime sandbox root commands must use `execRoot(...)` with a `RootCommand`. Do not pass
`{ user: "root" }` to generic `sandbox.exec(...)` / `provider.exec(...)`; that path is reserved for
non-root sandbox workloads. Do not bypass the provider with direct E2B SDK root execution
(`sandbox.commands.run(..., { user: "root" })`, `sandbox.pty.create(..., { user: "root" })`) outside
the provider implementation or explicitly operator-only scripts. Prefer
`rootCommand.exec("/absolute/path", args)` and the small `rootCommand` combinators. Use
`rootCommand.unsafeShell(command, reason)` only for compound shell flows that cannot reasonably be
expressed with the builder, and include a specific reason.
When passing positional operands that can be influenced by sandbox workload data or other untrusted
input, account for option injection. If the target executable supports it, insert `--` before those
operands so values beginning with `-` cannot be interpreted as flags.
Do not rely on the sandbox `PATH` for root commands. Sandbox workloads may control writable
directories that appear in a default root PATH on some images, so bare commands like `cat`,
`chmod`, `nohup`, `systemctl`, `tail`, `head`, `env`, or project helper names can become root code
execution if an unprivileged sandbox user can plant a matching file earlier in PATH.
If root invokes a helper by absolute path, the helper and every parent directory that makes it
reachable must be root-owned and not group/other writable before the helper is trusted. Prefer
root-owned locations such as `/opt/bin` for Dust-managed sandbox helpers, and add image/runtime
hardening plus tests for their ownership and mode.
Reviewer: If you detect a runtime sandbox command using `sandbox.exec(..., { user: "root" })` or
`provider.exec(..., { user: "root" })`, require `execRoot(...)` and a `RootCommand`. If you detect
direct E2B SDK root execution outside the provider or operator-only scripts, require routing
through the provider. If you detect `rootCommand.unsafeShell(...)`, check that the reason is
concrete and that simple absolute-executable builder calls would not be clearer. If root command
args include untrusted positional operands without a `--` separator, check whether the target
executable can interpret them as options. If you detect a root-invoked helper in a path that sandbox
users can write to, require ownership/mode hardening or a safer location before approving.
Example:
```
// BAD: runtime root through generic exec, with commands resolved through PATH.
await sandbox.exec(auth, "cat /tmp/deny.log | wc -l | tr -d ' '", {
user: "root",
});
// GOOD: root execution uses execRoot and a RootCommand.
await sandbox.execRoot(
auth,
rootCommand.and([
rootCommand.exec("/usr/bin/install", ["-o", "root", "-g", "root", "-m", "600", "/dev/stdin", path]),
rootCommand.exec("/usr/bin/mv", [path, finalPath]),
])
);
// BAD: root invokes a helper from a path whose ownership/mode is not enforced.
await sandbox.execRoot(auth, rootCommand.unsafeShell("dust-install-trust-bundle", "bad example"));
// GOOD: root invokes an absolute helper path that image/runtime hardening keeps root-owned and
// non-writable by sandbox workloads.
await sandbox.execRoot(auth, rootCommand.exec("/opt/bin/dust-install-trust-bundle"));
```
@cc [label:security] sandbox-privileged-directory-ownership
Sandbox workloads must not be able to write to any directory that privileged sandbox services use
for discovery or activation. This includes systemd unit directories, tmpfiles/profile directories,
dynamic library lookup directories, plugin directories, and every parent directory that controls
those paths.
Treat this separately from command `PATH` hardening: root can execute attacker-controlled code by
loading a systemd unit, tmpfiles config, profile script, library, or plugin from a higher-precedence
lookup directory even when every command uses absolute executable paths.
Reviewer: If sandbox code adds or depends on a root-consumed lookup path, require image/runtime
hardening that makes the path root-owned and not group/other writable, plus a regression test that
attempts to write the attacker-controlled file as the workload user.
@cc [owner:spolu,label:error-handling] no-catching-own-errors
Repository functions MUST return failures that callers are expected to handle as `Err<E>` values in
`Result<T, E>`, rather than throw them.
Applicable local contracts MAY define explicit exceptions to this rule, limited to the boundaries
and failure cases they specify; callers MAY catch those documented failures using runtime type
guards. `catch` is also permitted around external-library calls whose exceptions we do not control.
Outside these exceptions, errors thrown by repository code MUST propagate to the runtime's error
handler.
@cc [owner:spolu,label:error-handling] normalize-caught-errors
Caught values MUST NOT be treated as `Error` or an error subtype through a type assertion. When an
`Error` is required, code MUST use `normalizeError(caught)` unless a runtime type guard has already
established the required type. Inspecting a caught value with a type guard or rethrowing it
unchanged does not require normalization.
```typescript
// BAD: A type assertion does not validate the caught value.
try {
await externalClient.request();
} catch (err) {
return new Err(err as Error);
}
// GOOD: Normalize the caught value before returning it as an error.
try {
await externalClient.request();
} catch (err) {
return new Err(normalizeError(err));
}
```