Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions apps/api/src/lib/cookies/cookie-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,23 @@ export const REFRESH_COOKIE_NAME = "refresh_token";

const REFRESH_COOKIE_MAX_AGE_SECONDS = 30 * 24 * 60 * 60;

/*
* `secure` is always true: browsers treat http://localhost as a secure
* context, so dev still works, while staging/preview hosts never ship auth
* cookies over plaintext. `sameSite` stays env-conditional because dev OAuth
* redirects cross ports and need `lax`.
*/
export const AUTH_COOKIE_CONFIG = {
httpOnly: true,
secure: env.isProduction,
secure: true,
sameSite: env.isProduction ? ("strict" as const) : ("lax" as const),
maxAge: JWT_TTL_SECONDS,
path: "/",
};

export const REFRESH_COOKIE_CONFIG = {
httpOnly: true,
secure: env.isProduction,
secure: true,
sameSite: env.isProduction ? ("strict" as const) : ("lax" as const),
maxAge: REFRESH_COOKIE_MAX_AGE_SECONDS,
path: "/",
Expand Down
33 changes: 33 additions & 0 deletions apps/api/src/lib/rate-limit/valkey-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,23 @@ interface IIncrementResult {

const RATE_LIMIT_KEY_PREFIX = "rl:";

/*
* A non-positive window means rate limiting is failing open on every
* request. `init()` already warns once at startup, but startup logs roll
* off and the open state would then be invisible to alerting. Re-emit the
* warning from the hot increment path, throttled to once per interval so a
* persistent misconfiguration keeps an alert rule firing without flooding
* the log on every request.
*/
const DISABLED_WARN_INTERVAL_MS = 60_000;

const buildKey = (rawKey: string): string =>
`${RATE_LIMIT_KEY_PREFIX}${rawKey}`;

export class ValkeyRateLimitContext implements RateLimitContext {
private readonly client: Redis;
private durationMs = 0;
private lastDisabledWarnMs = Number.NEGATIVE_INFINITY;

constructor(client: Redis = new Redis(getValkeyAppClientOptions())) {
this.client = client;
Expand Down Expand Up @@ -93,6 +104,8 @@ export class ValkeyRateLimitContext implements RateLimitContext {
const durationMs = duration ?? this.durationMs;

if (durationMs <= 0) {
this.warnRateLimitingDisabled(now, durationMs);

return this.permissiveFallback(now, durationMs);
}

Expand Down Expand Up @@ -189,6 +202,26 @@ export class ValkeyRateLimitContext implements RateLimitContext {
this.client.disconnect();
}

/*
* Surface a non-positive window from the request path, throttled so a
* persistent misconfiguration stays visible to alerting without flooding
* the log. `now` is the request clock, so the throttle is deterministic.
*/
private warnRateLimitingDisabled(now: number, durationMs: number): void {
if (now - this.lastDisabledWarnMs < DISABLED_WARN_INTERVAL_MS) {
return;
}

this.lastDisabledWarnMs = now;
logger.warn(
"Rate-limit window is non-positive; Valkey rate limiting is failing open",
{
event: "rate_limit_misconfigured",
durationMs,
}
);
}

/**
* Return a "first request in window" shape so the caller treats the
* request as allowed. We choose permissive over restrictive on
Expand Down
8 changes: 8 additions & 0 deletions apps/api/tests/lib/cookies/cookie-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ describe("AUTH_COOKIE_CONFIG", () => {
test("sameSite is one of the safe lax/strict values", () => {
expect(["lax", "strict"]).toContain(AUTH_COOKIE_CONFIG.sameSite);
});

test("is always Secure (never shipped over plaintext)", () => {
expect(AUTH_COOKIE_CONFIG.secure).toBe(true);
});
});

describe("REFRESH_COOKIE_CONFIG", () => {
Expand All @@ -49,4 +53,8 @@ describe("REFRESH_COOKIE_CONFIG", () => {
AUTH_COOKIE_CONFIG.maxAge
);
});

test("is always Secure (never shipped over plaintext)", () => {
expect(REFRESH_COOKIE_CONFIG.secure).toBe(true);
});
});
50 changes: 50 additions & 0 deletions apps/api/tests/lib/rate-limit/valkey-context.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, expect, spyOn, test } from "bun:test";
import { Redis } from "ioredis";

import { logger } from "../../../src/config/logger";
import { ValkeyRateLimitContext } from "../../../src/lib/rate-limit/valkey-context";

/*
* `lazyConnect` keeps ioredis from opening a socket: the misconfiguration
* path returns before issuing any command, so the client is never touched.
*/
const makeContext = (): { ctx: ValkeyRateLimitContext; client: Redis } => {
const client = new Redis({ lazyConnect: true });

return { ctx: new ValkeyRateLimitContext(client), client };
};

const countMisconfigWarns = (calls: unknown[]): number =>
(JSON.stringify(calls).match(/rate_limit_misconfigured/gu) ?? []).length;

describe("ValkeyRateLimitContext non-positive window", () => {
test("fails open and warns when the window is non-positive", async () => {
const { ctx, client } = makeContext();
const warnSpy = spyOn(logger, "warn");

try {
const result = await ctx.increment("user:1", 0, 1_000);

expect(result.count).toBe(1);
expect(countMisconfigWarns(warnSpy.mock.calls)).toBe(1);
} finally {
warnSpy.mockRestore();
client.disconnect();
}
});

test("throttles the warning so the hot path does not flood the log", async () => {
const { ctx, client } = makeContext();
const warnSpy = spyOn(logger, "warn");

try {
await ctx.increment("user:1", 0, 1_000);
await ctx.increment("user:1", 0, 1_500);

expect(countMisconfigWarns(warnSpy.mock.calls)).toBe(1);
} finally {
warnSpy.mockRestore();
client.disconnect();
}
});
});
12 changes: 12 additions & 0 deletions apps/docs/src/data/lint-meta-catalog.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@
"ciCritical": false,
"description": "Use the typed apiClient; raw fetch is restricted to src/lib/api/openapi."
},
{
"id": "no-inline-object-cast",
"category": "source-text",
"ciCritical": false,
"description": "Casting to an inline object type (`as { … }`) skips validation."
},
{
"id": "no-dark-variant",
"category": "source-text",
Expand Down Expand Up @@ -138,6 +144,12 @@
"ciCritical": false,
"description": "Logic modules must ship with a colocated *.test.ts or *.test.tsx sibling."
},
{
"id": "test-files-require-source-sibling",
"category": "testing",
"ciCritical": false,
"description": "Colocated test files must mirror a source sibling (no orphaned tests)."
},
{
"id": "skipped-tests-need-tracking",
"category": "testing",
Expand Down
2 changes: 2 additions & 0 deletions apps/ui/scripts/lint-meta/RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ Run `bun run lint:meta --list-rules` for the machine-readable list from the regi
| `no-dangerous-html` | source-text | no | Raw HTML rendering requires a dedicated sanitizer and security review. |
| `env-access` | source-text | no | Read Vite env through src/lib/env only. |
| `no-raw-fetch` | source-text | no | Use the typed apiClient; raw fetch is restricted to src/lib/api/openapi. |
| `no-inline-object-cast` | source-text | no | Casting to an inline object type (`as { … }`) skips validation. |
| `no-dark-variant` | source-text | no | The `dark:` Tailwind variant is banned. |
| `no-cross-repo-import` | source-text | **yes** | Relative imports must stay inside apps/ui; no backend or infra source paths. |
| `no-raw-role-literal` | source-text | no | Use ROLE.* from acl.types instead of raw owner/admin/member/viewer string literals. |
| `no-raw-fetch-scripts` | source-text | no | Scripts must not call global fetch except github-actions-permissions.ts (lint:meta --verify SHA check). |
| `queries-no-silent-error-swallow` | source-text | no | *.queries.ts files must not silently swallow query errors as `null`. Let the typed error propagate so consumers can distinguish auth from outage; opt-out per-catch with `// allow-silent: <reason>` when an explicit null is genuinely the right contract. |
| `logic-files-require-test-sibling` | testing | no | Logic modules must ship with a colocated *.test.ts or *.test.tsx sibling. |
| `test-files-require-source-sibling` | testing | no | Colocated test files must mirror a source sibling (no orphaned tests). |
| `skipped-tests-need-tracking` | testing | no | Skipped tests (.skip/.only/xit/xdescribe) must carry an issue URL or TODO(@owner) so the debt has a tracked owner. |
| `eslint-config-no-warn` | config | no | ESLint severities must be "error" or "off", not "warn". |

Expand Down
1 change: 1 addition & 0 deletions apps/ui/scripts/lint-meta/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export { checkCanonicalHelpersSingleHome } from "./rules/source-text/canonical-h
export { checkNoCrossRepoImports } from "./rules/source-text/no-cross-repo-import";
export { checkNoRawRoleLiterals } from "./rules/source-text/no-raw-role-literals";
export { checkScriptRawFetch } from "./rules/source-text/script-raw-fetch";
export { checkTestFilesHaveSource } from "./rules/testing/test-files-require-source-sibling";

/** @param file Absolute path to the source file under test */
export function checkForbiddenText(
Expand Down
2 changes: 2 additions & 0 deletions apps/ui/scripts/lint-meta/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { noOverlappingLibsRule } from "./rules/supply-chain/no-overlapping-libs"
import { packageJsonExactDepsRule } from "./rules/supply-chain/package-json-exact-deps";
import { logicFilesRequireTestSiblingRule } from "./rules/testing/logic-files-require-test-sibling";
import { skippedTestsNeedTrackingRule } from "./rules/testing/skipped-tests-need-tracking";
import { testFilesRequireSourceSiblingRule } from "./rules/testing/test-files-require-source-sibling";
import type { IMetaRule } from "./types";

export const META_RULES: readonly IMetaRule[] = [
Expand All @@ -41,6 +42,7 @@ export const META_RULES: readonly IMetaRule[] = [
noSilentErrorSwallowRule,
// --- testing ---
logicFilesRequireTestSiblingRule,
testFilesRequireSourceSiblingRule,
skippedTestsNeedTrackingRule,
// --- config ---
eslintConfigNoWarnRule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,27 @@ export function createForbiddenTextPatterns(
"Use the typed apiClient; raw fetch is restricted to src/lib/api/openapi.ts.",
allow: (file) => rawFetchAllowlist.has(file)
},
{
/*
* Casting a value to an inline object type (`x as { … }`) skips
* runtime validation — the classic footgun is asserting the shape of
* a parsed JSON / API response body and trusting it. The merge bar is
* "only `as const`"; ESLint's consistent-type-assertions only bans
* object-literal *expressions* (`{} as T`), not assertions *to* an
* inline object type, so this closes that gap for production source.
* Narrow the value with a type guard instead (see
* src/lib/api/openapi.ts `extractApiErrorBody`). Tests, e2e, and
* Storybook keep casting for fixtures, so the ban is src-only and
* skips colocated `*.test.*` files.
*/
rule: "no-inline-object-cast",
pattern: /\bas\s+\{/u,
message:
"Casting to an inline object type (`as { … }`) skips validation. Narrow the value with a type guard instead.",
allow: (file) =>
!file.startsWith(resolve(root, "src")) ||
/\.test\.(?:ts|tsx)$/u.test(file)
},
{
/*
* Theme tokens in tailwind.css are the only source of truth for
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { existsSync } from "node:fs";
import { join } from "node:path";

import { collectSourceFiles } from "../../context";
import type { IMetaRule, IViolation } from "../../types";

/*
* The complement of `logic-files-require-test-sibling`: every colocated
* `*.test.ts` / `*.test.tsx` under `src/` must sit next to the source file
* it covers. Catches orphaned tests left behind after a refactor or rename
* (a stray test silently keeps passing while the thing it claimed to cover
* is gone). The UI app colocates tests beside source, so this lives in
* lint-meta rather than the ESLint `test-conventions/test-file-mirrors-source`
* rule, which assumes a separate `tests/` tree and `.ts`-only sources.
*
* Tests under `tests/` (factories, lint-meta, service-worker suites) are
* intentionally not source-mirrored and are out of scope: this rule only
* walks `src/`.
*/
export function checkTestFilesHaveSource(root: string): IViolation[] {
const violations: IViolation[] = [];
const srcRoot = join(root, "src");

for (const file of collectSourceFiles(srcRoot)) {
let base: string | null = null;

if (file.endsWith(".test.ts")) {
base = file.slice(0, -".test.ts".length);
} else if (file.endsWith(".test.tsx")) {
base = file.slice(0, -".test.tsx".length);
}

if (base === null) {
continue;
}

if (existsSync(`${base}.ts`) || existsSync(`${base}.tsx`)) {
continue;
}

violations.push({
file,
rule: "test-files-require-source-sibling",
message: `Orphaned test. No source sibling found — expected \`${base.slice(
root.length + 1
)}.ts\` (or \`.tsx\`) next to this test. Rename the test to mirror the module it covers, move it beside that module, or delete it.`
});
}

return violations;
}

/** Colocated *.test.ts / *.test.tsx files must mirror a source sibling. */
export const testFilesRequireSourceSiblingRule: IMetaRule = {
id: "test-files-require-source-sibling",
category: "testing",
description:
"Colocated test files must mirror a source sibling (no orphaned tests).",
run({ root }) {
return checkTestFilesHaveSource(root);
}
};
6 changes: 5 additions & 1 deletion apps/ui/src/features/auth/Auth.signup.mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ export function useRegister(): UseMutationResult<
throw new ApiError(0, { message: "Empty register response" });
}

const message = (data.data as { message?: string }).message;
const payload: unknown = data.data;
const message =
typeof payload === "object" && payload !== null && "message" in payload
? Reflect.get(payload, "message")
: undefined;

return typeof message === "string" ? message : "";
}
Expand Down
14 changes: 10 additions & 4 deletions apps/ui/src/lib/api/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,18 @@ let inFlightRefresh: Promise<boolean> | null = null;
* reconnect loop indefinitely after logout because the anonymous probe is
* also a 200.
*/
function getProp(value: unknown, key: string): unknown {
if (typeof value !== "object" || value === null || !(key in value)) {
return undefined;
}

return Reflect.get(value, key);
}

async function readSessionUserId(response: Response): Promise<string | null> {
try {
const body = (await response.clone().json()) as {
data?: { user?: { id?: unknown } | null } | null;
} | null;
const userId = body?.data?.user?.id;
const body: unknown = await response.clone().json();
const userId = getProp(getProp(getProp(body, "data"), "user"), "id");

if (typeof userId === "string" && userId.length > 0) {
return userId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { sanitizeTargetPath } from "@/lib/web-push/sw-url-sanitize";
*/
const ORIGIN = "https://app.example.test";

describe("in-app notification CTA sanitization", () => {
describe("sanitizeTargetPath (in-app notification CTA sanitization)", () => {
it("preserves a same-origin path", () => {
expect(sanitizeTargetPath("/notifications/123", ORIGIN)).toBe(
"/notifications/123"
Expand Down
Loading
Loading