---
title: Security Audit — Qgen (Batappli Quote Generator API)
description: A full security review of Qgen, a NestJS LLM API: 28 findings, 3 critical, 8 high, and the remediation order.
date: 2026-09-04
lang: en-US
author: Julien Béranger
model: Claude Opus 5
source: https://julienberanger.com/qgen-security-audit-sept-4-2026
---

# Security Audit — Qgen (Batappli Quote Generator API)

| | |
|---|---|
| **Application** | Qgen v0.2.0 — NestJS 11 REST API |
| **Repository** | `/Users/ju/qgen` |
| **Branch audited** | `63-improve-instructions-for-surface-calculation` (HEAD `6a57364`) |
| **Audit date** | 4 September 2026 |
| **Scope** | Full application source (`src/`, 58 TypeScript files), dependency tree, configuration, CI pipeline, deployment posture |
| **Methodology** | Manual source review, data-flow tracing from HTTP entry points to filesystem/network sinks, dependency advisory analysis (`pnpm audit`), and empirical verification of exploit preconditions in a local harness |
| **Overall rating** | **Critical — not suitable for untrusted or internet-facing exposure in its current state** |

---

## 1. Executive Summary

Qgen is a NestJS API that accepts natural-language requests, enriches them with markdown "context" files retrieved from disk, and forwards them to one of four LLM providers (Anthropic, OpenAI, Mistral, DeepSeek). It also exposes a context-management CRUD surface, a headless-browser web reader, cost tracking, and log/report endpoints.

The review identified **28 findings**, of which **3 are Critical** and **8 are High**.

The central problem is that the application performs **no path sanitisation anywhere in the codebase**. Every filesystem operation is built by concatenating attacker-controlled strings into `path.join()` calls, and no call to `path.resolve()`, `path.normalize()`, or `path.basename()` exists in `src/`. Because Express 5 URL-decodes route parameters, an encoded `%2F` in a path segment becomes a real directory separator, which converts three routes into arbitrary file read, arbitrary file write, and arbitrary recursive deletion primitives.

These primitives are catastrophic in this specific application because its most sensitive assets are plain files on the same disk: `data/api-keys.json` holds every API key in cleartext, and `.env` holds four LLM provider keys, a GitHub token, the `MASTER_KEY`, and an Ethereum `PRIVATE_KEY`. A single authenticated request therefore escalates to full compromise of the service and of the third-party accounts it bills against.

Compounding this, the authorisation model is binary: any valid API key is fully privileged. There is no per-key ownership, no scoping, and no rate limiting of any kind. `GET /logs` returns every user's messages and session identifiers to any key holder, and because conversation memory is keyed solely on a client-supplied `sessionId`, those leaked identifiers permit replay of other customers' conversations.

**Priority remediation order:**

1. Sanitise every filesystem path (Findings C-01, C-02, H-02, M-01) — one shared helper resolves all four.
2. Hash API keys at rest and stop rewriting the key store on every request (C-03, H-06).
3. Add an SSRF allowlist to the web reader and restore the Chrome sandbox (H-01).
4. Introduce rate limiting, particularly on the unauthenticated master-key endpoint (H-05).
5. Introduce per-key ownership for logs, sessions, and context mutations (H-03, H-04).
6. Terminate TLS in front of the service; the documented production endpoint is plaintext HTTP (M-06).

### 1.1 Findings by severity

| Severity | Count | Finding IDs |
|---|---|---|
| Critical | 3 | C-01, C-02, C-03 |
| High | 8 | H-01 … H-08 |
| Medium | 9 | M-01 … M-09 |
| Low | 8 | L-01 … L-08 |
| **Total** | **28** | |

### 1.2 Findings index

| ID | Severity | Title | Primary location |
|---|---|---|---|
| C-01 | Critical | Path traversal enables arbitrary file read | [context.controller.ts:180](src/context/context.controller.ts#L180) |
| C-02 | Critical | Path traversal enables arbitrary file and directory deletion | [context.controller.ts:290](src/context/context.controller.ts#L290) |
| C-03 | Critical | API keys stored in cleartext and served from a readable path | [api-key.service.ts:58](src/auth/api-key.service.ts#L58) |
| H-01 | High | Server-side request forgery in the web reader | [web-reader.service.ts:56-84](src/web/web-reader.service.ts#L56-L84) |
| H-02 | High | Arbitrary `.md` file write via unsanitised upload filename | [context.controller.ts:226](src/context/context.controller.ts#L226) |
| H-03 | High | No authorisation model; all API keys are fully privileged | [api-key.guard.ts:19](src/auth/api-key.guard.ts#L19) |
| H-04 | High | Session hijacking via unbound, client-supplied `sessionId` | [app.service.ts:391](src/app.service.ts#L391) |
| H-05 | High | No rate limiting; unauthenticated master-key brute force | [auth.controller.ts:29](src/auth/auth.controller.ts#L29) |
| H-06 | High | Key store rewritten on every request (race condition, DoS) | [api-key.service.ts:94](src/auth/api-key.service.ts#L94) |
| H-07 | High | GraphQL injection in GitHub sponsorship check | [subs.service.ts:102](src/subs/subs.service.ts#L102) |
| H-08 | High | 49 known vulnerabilities in production dependencies | `package.json` |
| M-01 | Medium | Context traversal in `POST /ask` leaks files through model output | [app.service.ts:383](src/app.service.ts#L383) |
| M-02 | Medium | Unbounded upload buffering before size validation | [app.controller.ts:163](src/app.controller.ts#L163) |
| M-03 | Medium | Error text and stack traces published to a public ntfy.sh topic | [notification-logger.service.ts:39](src/notification-logger.service.ts#L39) |
| M-04 | Medium | No security headers, no CORS policy, Swagger exposed unauthenticated | [main.ts:39](src/main.ts#L39) |
| M-05 | Medium | Non-constant-time comparison of API keys and master key | [api-key.service.ts:86](src/auth/api-key.service.ts#L86) |
| M-06 | Medium | Production endpoint documented as plaintext HTTP | `package.json` scripts |
| M-07 | Medium | Indirect prompt injection through context files and fetched links | [app.service.ts:334](src/app.service.ts#L334) |
| M-08 | Medium | Key deletion by 12-character prefix can remove the wrong key | [api-key.service.ts:116](src/auth/api-key.service.ts#L116) |
| M-09 | Medium | Unsynchronised read-modify-write on shared JSON stores | [json-store.ts:11](src/storage/json-store.ts#L11) |
| L-01 | Low | Secret files are world-readable (mode 0644) | `.env`, `data/*.json` |
| L-02 | Low | `forbidNonWhitelisted` disabled on the global validation pipe | [main.ts:19](src/main.ts#L19) |
| L-03 | Low | Internal error messages propagated to clients | [context.controller.ts:75-78](src/context/context.controller.ts#L75-L78) |
| L-04 | Low | `Math.random()` used in request identifiers | [app.service.ts:389](src/app.service.ts#L389) |
| L-05 | Low | Core business logic excluded from version control | `.gitignore` |
| L-06 | Low | CI performs no dependency scanning and no frozen-lockfile install | `.github/workflows/test.yml` |
| L-07 | Low | Weak default master key in the environment template | `.env.template` |
| L-08 | Low | Indefinite retention of customer conversation content | [custom-memory.ts:67-89](src/memory/custom-memory.ts#L67-L89) |

---

## 2. System Overview

### 2.1 Attack surface

| Route | Method | Authentication | Notes |
|---|---|---|---|
| `/` | GET | Public | Static HTML landing page |
| `/api` | GET | **None** | Swagger UI, registered outside the guard chain |
| `/auth/api-keys` | POST | **None** (`@Public`) | Master-key-gated key issuance |
| `/auth/api-keys` | GET | API key | Lists all keys with previews |
| `/ask` | POST | API key | Core LLM endpoint; accepts a file upload |
| `/feedback` | POST | API key | Mutates any log entry by `requestId` |
| `/logs`, `/logs/:requestId` | GET | API key | Returns all users' request history |
| `/context` | POST, GET | API key | Create / list contexts |
| `/context/:name/files` | GET | API key | List files |
| `/context/:name/file/:filename` | GET | API key | **Arbitrary file read (C-01)** |
| `/context/upload` | POST | API key | **Arbitrary `.md` write (H-02)** |
| `/context/:name` | DELETE | API key | **Arbitrary directory deletion (C-02)** |
| `/context/:name/file` | DELETE | API key | **Arbitrary file deletion (C-02)** |
| `/context/:name/link`, `/links` | POST, GET, DELETE | API key | Registers URLs later fetched server-side |
| `/web-reader`, `/web-reader/llm` | GET | API key | **SSRF (H-01)** |
| `/reports/weekly/generate` | POST | API key | Triggers report generation and log truncation |

### 2.2 Trust boundaries and sensitive assets

The application stores all state as JSON files under `data/` and reads its secrets from `.env` in the process working directory. Both directories sit inside the traversal reach of the context routes.

| Asset | Location | Exposure |
|---|---|---|
| All issued API keys (cleartext) | `data/api-keys.json` | Readable via C-01 |
| LLM provider keys ×4, `GITHUB_API_TOKEN`, `NTFY_TOKEN`, `MASTER_KEY`, Ethereum `PRIVATE_KEY` | `.env` | Readable via C-01 |
| Customer conversation transcripts | `data/chat-history.json` | Readable via C-01; replayable via H-04 |
| Request logs incl. user identifiers | `data/logs.json` | Exposed by `GET /logs` to any key holder |
| Cost/billing records | `data/costs.json` | Readable via C-01 |

---

## 3. Critical Findings

### C-01 — Path traversal enables arbitrary file read

| | |
|---|---|
| **Severity** | Critical |
| **CWE** | CWE-22: Improper Limitation of a Pathname to a Restricted Directory |
| **CVSS 3.1 (est.)** | 8.7 — `AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N` |
| **Location** | [context.controller.ts:157-190](src/context/context.controller.ts#L157-L190) → [context.service.ts:317-341](src/context/context.service.ts#L317-L341) |
| **Status** | Verified empirically |

**Description.** The route handler `GET /context/:name/file/:filename` passes both parameters directly to `ContextService.getFileContent`, which builds the target path by concatenation:

```ts
// src/context/context.service.ts:317
async getFileContent(contextName: string, fileName: string): Promise<string> {
  const contextPath = join(this.contextsPath, contextName);
  const filePath = join(contextPath, fileName);   // no normalisation, no containment check
  ...
  return await readFile(filePath, 'utf-8');
}
```

Neither parameter is validated. The `@Matches(/^[a-z0-9-]+$/)` constraint that exists on `CreateContextDto` is not applied to route parameters, and there is no `.md` extension constraint on the read path.

Express 5 (`express@5.2.1`, `path-to-regexp@8.3.0`) matches a route segment against the raw URL and then applies `decodeURIComponent` to the captured value. A percent-encoded `%2F` therefore does not break segment matching, but does become a literal `/` inside `req.params`. This was confirmed against the project's own installed Express version:

```
GET /context/batappli/file/..%2F..%2F..%2F..%2Fetc%2Fpasswd
  → params.filename = "../../../../etc/passwd"
  → resolved = /srv/etc/passwd

GET /context/batappli/file/%2e%2e%2f%2e%2e%2fapi-keys.json
  → params.filename = "../../api-keys.json"
  → resolved = /srv/app/data/api-keys.json
```

The only precondition is that `data/contexts/<name>` exists; the `batappli` context is committed to the repository, so this is satisfied by default on every deployment.

**Impact.** Any holder of any valid API key — including a key legitimately issued to a low-trust integration — can read any file the Node process can read. The two decisive targets are:

- `data/api-keys.json`, which yields **every API key in the system in cleartext** (see C-03), removing all remaining access control.
- `.env`, which yields the Anthropic, OpenAI, Mistral and DeepSeek API keys, the `GITHUB_API_TOKEN`, the `NTFY_TOKEN`, the `MASTER_KEY` (enabling unlimited key issuance), and an Ethereum `PRIVATE_KEY`.

Beyond the application, this reaches SSH keys, cloud instance credentials, and any other file readable by the service account. Recovery requires rotating all provider credentials and any funds controlled by the exposed private key.

**Remediation.** Introduce a single containment helper and apply it to every path built from user input:

```ts
import { resolve, sep } from 'path';

private safeJoin(base: string, ...segments: string[]): string {
  const target = resolve(base, ...segments);
  const root = resolve(base);
  if (target !== root && !target.startsWith(root + sep)) {
    throw new BadRequestException('Invalid path');
  }
  return target;
}
```

In addition: validate `:name` against `/^[a-z0-9-]+$/` and `:filename` against `/^[a-zA-Z0-9._-]+\.md$/` with a class-validator DTO on the parameters, and reject any value containing `/`, `\`, or `..` before the path is constructed. Defence in depth: run the service under a dedicated unprivileged account whose read access is limited to the application directory, and load secrets from a secret manager rather than a file adjacent to the data directory.

---

### C-02 — Path traversal enables arbitrary file and directory deletion

| | |
|---|---|
| **Severity** | Critical |
| **CWE** | CWE-22 / CWE-73: External Control of File Name or Path |
| **CVSS 3.1 (est.)** | 8.1 — `AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H` |
| **Location** | [context.service.ts:99-112](src/context/context.service.ts#L99-L112), [context.service.ts:203-226](src/context/context.service.ts#L203-L226) |
| **Status** | Verified empirically |

**Description.** Two deletion paths share the flaw described in C-01, with write-side consequences.

`DELETE /context/:name` reaches `deleteContext`, which performs a **recursive** removal on an unvalidated, concatenated path:

```ts
// src/context/context.service.ts:99
async deleteContext(name: string): Promise<void> {
  const contextPath = join(this.contextsPath, name);
  if (!existsSync(contextPath)) throw new Error(`Context '${name}' not found`);
  await rm(contextPath, { recursive: true });
}
```

Verified decoding: `DELETE /context/..%2F..%2F..%2Ftmp%2Fvictim` resolves `params.name` to `../../../tmp/victim` and the target to `/srv/tmp/victim`. Any directory tree the process can write is deletable in a single request.

`DELETE /context/:name/file` reaches `deleteFile`, whose `filename` comes from `DeleteFileDto`, constrained only by `@IsString()` and `@IsNotEmpty()` — there is no extension or path constraint — and is passed to `rm(filePath)`.

**Impact.** Destruction of application data (`data/api-keys.json`, `data/chat-history.json`, `data/logs.json`, the entire `data/contexts/` corpus), of the deployed application itself (`dist/`, `node_modules/`), or of unrelated data on the same host. Deleting `data/api-keys.json` is a denial-of-service that locks out every legitimate integration, since `loadApiKeys` returns `[]` on a missing file and every subsequent request fails authentication. There is no backup mechanism in the codebase and no audit trail beyond a log line.

**Remediation.** Apply the `safeJoin` helper from C-01 to both methods. Constrain `:name` to the context-name character class and `filename` to a basename with a `.md` extension. Consider replacing `rm(..., { recursive: true })` with an explicit enumerate-and-unlink over known index entries, so that deletion can only ever touch files the context index actually records.

---

### C-03 — API keys stored in cleartext and served from a readable path

| | |
|---|---|
| **Severity** | Critical |
| **CWE** | CWE-256: Plaintext Storage of a Password; CWE-522: Insufficiently Protected Credentials |
| **CVSS 3.1 (est.)** | 7.5 (standalone) — escalates to 9.1 when chained with C-01 |
| **Location** | [api-key.service.ts:58-99](src/auth/api-key.service.ts#L58-L99) |

**Description.** Keys are generated correctly — `qgen_` plus 32 bytes from `crypto.randomBytes`, which is 256 bits of entropy and not brute-forceable — but are then persisted verbatim:

```ts
// src/auth/api-key.service.ts:66
const newKey: ApiKey = { key: this.generateKey(), name, createdAt: ... };
apiKeys.push(newKey);
await this.saveApiKeys(apiKeys);   // written to data/api-keys.json in cleartext
```

Validation is a plaintext equality search (`apiKeys.find((k) => k.key === key)`). The file is written with default permissions; on the audited host it is mode `0644`, world-readable.

Because the credential store is a file inside `data/`, it is directly reachable by the traversal primitive in C-01 — the shortest full-compromise chain in the application is a single `GET` request.

**Impact.** Any read access to the filesystem — through C-01, through a backup, through a misconfigured volume mount, through another user on a shared host, or through an accidental commit — discloses every credential in a form that is immediately replayable. There is no key expiry, no revocation endpoint (`deleteApiKey` exists in the service but is never routed), and no per-key audit trail beyond a `lastUsed` timestamp.

**Remediation.**

1. Store only a hash of each key. Because these are high-entropy random tokens rather than user passwords, a single SHA-256 is cryptographically adequate and keeps validation fast; return the cleartext key exactly once at creation and never again.
2. Compare hashes with `crypto.timingSafeEqual` (see M-05).
3. Give the key file mode `0600` and place it outside any directory served or traversable by request handlers.
4. Add key expiry, an authenticated revocation endpoint, and a scope or owner field (see H-03).
5. Migrate to a datastore with access control if the deployment grows beyond a single node — the current design cannot support more than one instance safely in any case (see H-06).

---

## 4. High-Severity Findings

### H-01 — Server-side request forgery in the web reader

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-918: Server-Side Request Forgery |
| **CVSS 3.1 (est.)** | 8.6 — `AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:L/A:L` |
| **Location** | [web-reader.service.ts:47-95](src/web/web-reader.service.ts#L47-L95), [web-reader.controller.ts:14-16](src/web/web-reader.controller.ts#L14-L16) |
| **Status** | Verified empirically |

**Description.** `GET /web-reader?url=…` launches headless Chrome and navigates to a caller-supplied URL. The only validation is a syntactic `new URL(url)` parse in the service and a `@IsUrl()` decorator on the DTO. Neither restricts the destination host.

Testing `@IsUrl()`'s underlying `validator@13.15.26` implementation against representative internal targets:

| Target | Passes `@IsUrl()` |
|---|---|
| `http://169.254.169.254/latest/meta-data/` | **yes** |
| `http://127.0.0.1:3000/logs` | **yes** |
| `http://10.0.0.5:8080/admin` | **yes** |
| `http://192.168.1.1/` | **yes** |
| `http://[::1]:3000/` | **yes** |
| `http://metadata.google.internal/` | **yes** |
| `http://localhost:3000/` | no (TLD required) |
| `file:///etc/passwd` | no (scheme not permitted) |

The `localhost` and `file://` rejections are incidental side effects of the validator's defaults, not deliberate controls, and they fall away entirely on the second reachable path: URLs registered via `POST /context/:name/link` are stored in the context index and later fetched by `AppService.loadContextInformation` ([app.service.ts:334](src/app.service.ts#L334)) with no `@IsUrl` re-validation of the stored value.

The response body returns the fetched content to the caller, making this a **full-read SSRF** rather than a blind one.

Two aggravating factors:

- Chrome is launched with `--no-sandbox --disable-setuid-sandbox` ([web-reader.service.ts:68](src/web/web-reader.service.ts#L68)). Any renderer-level exploit delivered by a hostile page therefore executes directly as the service account, with no sandbox to escape.
- Redirects are followed by the browser, so a same-origin allowlist applied only to the initial URL would be insufficient.

**Impact.** Reads cloud instance metadata (on AWS IMDSv1, EC2 role credentials), reaches internal services not exposed to the internet, port-scans the internal network via timing and error differentials, and — because the app binds to `0.0.0.0:3000` — can address its own unauthenticated `/api` surface. Content fetched this way is also injected into LLM system prompts (see M-07).

**Remediation.**

1. Enforce an explicit **allowlist** of permitted hosts or domains; if an allowlist is impractical, resolve the hostname and reject any address in a private, loopback, link-local, or reserved range, re-checking after every redirect to defeat DNS rebinding and redirect chains.
2. Restrict schemes to `https:` (and `http:` only if genuinely required).
3. Remove `--no-sandbox`; if the deployment cannot support the Chrome sandbox, run the browser in a separate container with no network route to internal services and a read-only filesystem.
4. Apply the same validation to stored context links at fetch time, not only at registration time.
5. Cap response size and enforce a hard navigation timeout.

---

### H-02 — Arbitrary `.md` file write via unsanitised upload filename

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-434: Unrestricted Upload of File with Dangerous Type; CWE-22 |
| **CVSS 3.1 (est.)** | 8.1 — `AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:L` |
| **Location** | [context.controller.ts:203-272](src/context/context.controller.ts#L203-L272) → [context.service.ts:118-130](src/context/context.service.ts#L118-L130) |

**Description.** Two independent validation gaps combine here.

First, `UploadContextFileDto` declares `@Matches(/^[a-z0-9-]+$/)` on `contextName`, but the handler does not bind that DTO. It reads the raw field instead:

```ts
// src/context/context.controller.ts:226
async uploadFile(
  @Body('contextName') contextName: string,        // DTO constraints never applied
  @Body('fileDescription') fileDescription: string,
  @UploadedFile(new ParseFilePipe({ validators: [ new MaxFileSizeValidator({ maxSize: 5 * 1024 * 1024 }) ] }))
  file: Express.MulterFile,
) {
```

The `@ApiBody({ type: UploadContextFileDto })` decorator documents the DTO to Swagger but has no runtime validation effect. The regex is therefore dead code.

Second, the destination filename is taken from the multipart `originalname` — fully attacker-controlled — and is only checked for a `.md` suffix:

```ts
if (!file.originalname.toLowerCase().endsWith('.md')) throw new BadRequestException(...);
...
const filePath = join(contextPath, fileName);   // src/context/context.service.ts:125
await writeFile(filePath, content, 'utf-8');
```

A filename such as `../../../../srv/app/data/contexts/batappli/tarifs.md` satisfies the suffix check and escapes the context directory.

**Impact.** Write or overwrite any `.md` file the process can write. The `.md` restriction limits classic webshell scenarios but is precisely the wrong restriction for this application, because `.md` files **are** the trusted data: every file under `data/contexts/<name>/` is concatenated into the LLM system prompt. Overwriting a context file gives an attacker durable, server-side control over the instructions the model receives for every subsequent customer request — enabling manipulated pricing in generated quotes, exfiltration instructions, or arbitrary behaviour change, with no trace in the request logs. This is a persistent integrity compromise of the product's core function.

**Remediation.** Reduce the filename to its basename (`path.basename`), then validate it against `/^[a-zA-Z0-9._-]+\.md$/` and reject any leading dot. Apply `safeJoin` (C-01). Bind the actual DTO — `@Body() dto: UploadContextFileDto` — so the `contextName` regex is enforced at runtime. Validate the file's content type and not only its extension, and treat context-file mutation as a privileged operation gated by a scope (see H-03).

---

### H-03 — No authorisation model; all API keys are fully privileged

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-862: Missing Authorization; CWE-639: Authorization Bypass Through User-Controlled Key |
| **CVSS 3.1 (est.)** | 7.6 |
| **Location** | [api-key.guard.ts:19-44](src/auth/api-key.guard.ts#L19-L44), [app.controller.ts:215](src/app.controller.ts#L215) |

**Description.** `ApiKeyGuard` answers exactly one question — is this string present in `api-keys.json`? — and grants uniform access to every non-`@Public` route. The `ApiKey` interface carries `key`, `name`, `createdAt`, and `lastUsed`; there is no owner, tenant, scope, or role field, and the guard does not attach an identity to the request for downstream handlers to check.

Consequently a key issued for a narrow purpose confers the ability to:

- read every user's request history, messages, user identifiers and session identifiers via `GET /logs` ([app.controller.ts:215](src/app.controller.ts#L215)), which applies no filtering whatsoever;
- modify feedback on any other user's request via `POST /feedback`, which looks up `requestId` globally with no ownership check ([app.service.ts:899](src/app.service.ts#L899));
- enumerate all issued keys and their previews via `GET /auth/api-keys`;
- create, overwrite and recursively delete any context — the shared knowledge base for all customers;
- trigger `POST /reports/weekly/generate`, which **truncates `data/logs.json`** as a side effect ([weekly-report.service.ts:49-53](src/reports/weekly-report.service.ts#L49-L53)), destroying audit history on demand;
- exercise the traversal and SSRF primitives above.

The `userId` field on `AskDto` is caller-supplied, recorded verbatim, and never verified — it is a label, not an identity.

**Impact.** Any single compromised or over-shared integration key exposes all customers' data and permits destructive changes to shared state. In a multi-tenant deployment there is no isolation of any kind.

**Remediation.** Extend `ApiKey` with an owner identifier and a scope list (for example `ask`, `context:read`, `context:write`, `logs:read`, `admin`). Have the guard attach the resolved key record to `request.apiKey`, and introduce a `@Scopes(...)` decorator enforced by a second guard. Filter `GET /logs` and `GET /logs/:requestId` by owner. Verify ownership of `requestId` in `giveFeedback`. Restrict context mutation and report generation to an administrative scope.

---

### H-04 — Session hijacking via unbound, client-supplied `sessionId`

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-639: Authorization Bypass Through User-Controlled Key; CWE-384: Session Fixation |
| **CVSS 3.1 (est.)** | 7.1 |
| **Location** | [app.service.ts:391](src/app.service.ts#L391), [custom-memory.ts:62-66](src/memory/custom-memory.ts#L62-L66) |

**Description.** Conversation memory is partitioned solely on a string the client supplies:

```ts
// src/app.service.ts:391
let usedSessionId = sessionId || randomUUID();
```

`AskDto.sessionId` carries only `@IsOptional()` — not even `@IsString()`. `CustomJsonMemory` then filters the shared `data/chat-history.json` on equality with that value ([custom-memory.ts:65](src/memory/custom-memory.ts#L65)) and replays the matching turns into the next model call. Nothing binds a session to the API key, user, or origin that created it.

The identifiers are UUIDv4 and therefore not guessable in isolation — but they do not need to be guessed. `GET /logs` returns the `sessionId` of every request in the system to any key holder (H-03). The chain is:

1. Authenticate with any valid API key.
2. `GET /logs` → collect victims' `sessionId` values.
3. `POST /ask` with a victim's `sessionId` and a prompt such as "summarise our conversation so far".
4. The model replays the victim's prior turns — customer names, addresses, project details, quoted prices — into the attacker's response.

Session fixation is also possible in the other direction: an attacker who can influence a client's `sessionId` can pre-seed a conversation with instructions the victim's later turns will inherit.

**Impact.** Cross-customer disclosure of conversation content, which for this application means commercially sensitive quote data and personal information of French construction customers — squarely within GDPR scope.

**Remediation.** Bind every session to the identity that created it: store the owning key or user identifier alongside the session record and reject any request whose `sessionId` belongs to a different owner. Do not accept a `sessionId` for a session that does not exist — mint a new one instead of trusting the client's value. Add `@IsUUID()` validation. Segregate history storage per owner rather than filtering a single global file.

---

### H-05 — No rate limiting; unauthenticated master-key brute force

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-307: Improper Restriction of Excessive Authentication Attempts; CWE-770: Allocation Without Limits |
| **CVSS 3.1 (est.)** | 7.3 |
| **Location** | [auth.controller.ts:29-31](src/auth/auth.controller.ts#L29-L31); application-wide |

**Description.** The application declares no rate limiting: `@nestjs/throttler` is absent from `package.json`, and there is no `ThrottlerGuard`, middleware, or reverse-proxy configuration in the repository. The CHANGELOG records that a previous 50-requests-per-hour limit was **removed** (commit `35c15d3`), so this is a regression rather than an oversight.

Three distinct consequences follow.

*Credential attack.* `POST /auth/api-keys` is explicitly `@Public()` and gated only by a string comparison against `MASTER_KEY` ([auth.controller.ts:62](src/auth/auth.controller.ts#L62)). Unauthenticated callers may attempt it without limit. Failures are logged but never counted, throttled, or alerted on beyond a `logger.warn`. The environment template ships the placeholder `MASTER_KEY='super-secure-passphrase'` (L-07); any deployment retaining a human-memorable value is realistically brute-forceable. Success grants unlimited issuance of valid API keys.

*Financial denial of service.* `POST /ask` invokes paid LLM APIs with no per-key quota, no concurrency cap, and no spend ceiling. `CostTracker` records spend but never enforces a limit. A single key can be driven at maximum concurrency across four providers; the fallback loop at [app.service.ts:576](src/app.service.ts#L576) multiplies this by attempting **every** remaining provider when one fails, so an input crafted to fail (for example, one exceeding a provider's context window) costs up to four billed attempts per request.

*Resource denial of service.* `GET /web-reader` launches a **new Chrome process per request** with no pooling or concurrency limit ([web-reader.service.ts:65](src/web/web-reader.service.ts#L65)). A few dozen concurrent requests will exhaust host memory.

**Impact.** Unbounded third-party spend, host exhaustion, and an unmonitored path to master-key compromise and unlimited key issuance.

**Remediation.** Add `@nestjs/throttler` with a strict global default and a much stricter limit on `/auth/api-keys` (for example five attempts per hour per source address), plus lockout and alerting after repeated failures. Enforce per-key request and spend quotas in `CostTracker`, rejecting requests once a ceiling is reached. Bound the model fallback loop to at most one alternate provider, and only for retryable errors. Pool or cap concurrent browser instances. Enforce limits at the reverse proxy as well as in the application.

---

### H-06 — Key store rewritten on every request (race condition and DoS amplification)

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-362: Race Condition; CWE-400: Uncontrolled Resource Consumption |
| **CVSS 3.1 (est.)** | 7.0 |
| **Location** | [api-key.service.ts:84-97](src/auth/api-key.service.ts#L84-L97) |

**Description.** `validateApiKey` — executed on **every authenticated request** — reads the entire key file, mutates the matching record's `lastUsed`, and writes the whole file back:

```ts
async validateApiKey(key: string): Promise<boolean> {
  const apiKeys = await this.loadApiKeys();
  const apiKey = apiKeys.find((k) => k.key === key);
  if (!apiKey) return false;
  apiKey.lastUsed = new Date().toISOString();
  await this.saveApiKeys(apiKeys);   // full-file rewrite on the authentication hot path
  return true;
}
```

There is no locking, no atomic replace (no write-to-temp-then-rename), and no serialisation of concurrent writers. Two requests arriving concurrently both read the pre-state and both write their own version, so one update is lost. Worse, `writeFile` is not atomic: a crash, a container stop, or a disk-full condition part-way through the write leaves a truncated file. `loadApiKeys` catches the resulting JSON parse error and **returns an empty array**, which fails every subsequent authentication — a self-inflicted total outage that also silently discards the credential store, since the truncated file is the only copy.

A newly created key can also be lost outright: `createApiKey` and a concurrent `validateApiKey` both rewrite the same file from independently-read state.

**Impact.** Permanent loss of all API keys under a poorly-timed crash; write amplification proportional to request volume; a request-rate-controlled corruption window that an attacker can widen deliberately by driving concurrency (there is no rate limit to stop them — see H-05).

**Remediation.** Stop writing on the authentication path: keep the key set in memory, refresh it on change, and record `lastUsed` asynchronously to a separate append-only file or metrics sink. If file storage is retained, make every write atomic (write to a temporary file in the same directory, `fsync`, then `rename`) and serialise writers behind a mutex or advisory lock. For any multi-instance deployment, move the key store to a database — the current design cannot be run on more than one node without corrupting itself.

---

### H-07 — GraphQL injection in the GitHub sponsorship check

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-943: Improper Neutralization of Special Elements in Data Query Logic |
| **CVSS 3.1 (est.)** | 7.2 (conditional on reachability) |
| **Location** | [subs.service.ts:96-130](src/subs/subs.service.ts#L96-L130) |

**Description.** The GitHub GraphQL query is assembled by string interpolation of a caller-supplied username:

```ts
// src/subs/subs.service.ts:102
const query = `
  query {
    user(login: "${githubUsername}") {
      sponsoring(first: 100) { ... }
    }
  }
`;
```

`githubUserName` originates from the free-form `data` object on `AskDto`, which carries only `@IsOptional()` — no schema, no character restrictions. A value containing a double quote terminates the string literal and allows arbitrary GraphQL to be appended, executed against `api.github.com` **with the server's `GITHUB_API_TOKEN`** in the `Authorization` header.

**Reachability caveat.** `SubsService` is registered as a provider and injected into `AppService`, but `isSubscribed` is not currently called from any production code path — only from test doubles. The finding is therefore latent rather than actively exploitable at this commit. It is rated High because the vulnerable code is wired into the running application and a single future call site makes it live, and because the credential at risk is an organisation-scoped token.

**Impact.** If reached: arbitrary queries against GitHub with the service's token, disclosing private repository, organisation, and member data within the token's scopes; query-cost abuse; and, with a mutation-capable token, state changes on GitHub.

**Remediation.** Use GraphQL **variables** rather than interpolation:

```ts
body: JSON.stringify({
  query: 'query($login: String!) { user(login: $login) { ... } }',
  variables: { login: githubUsername },
})
```

Additionally, validate the username against GitHub's own format (`/^[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])){0,38}$/`), replace the untyped `data: Record<string, any>` with a validated DTO, and scope the token to the minimum required read permissions. If the sponsorship feature is genuinely dead code, delete the service rather than leaving it wired in.

---

### H-08 — Known vulnerabilities in production dependencies

| | |
|---|---|
| **Severity** | High |
| **CWE** | CWE-1395: Dependency on Vulnerable Third-Party Component |
| **Location** | `package.json`, `pnpm-lock.yaml` |

**Description.** `pnpm audit --prod` reports **49 advisories affecting runtime dependencies** (1 critical, 19 high, 24 moderate, 5 low). Across the full tree including development dependencies the total is 94 (2 critical, 50 high).

Runtime-reachable highlights:

| Package | Path | Advisory class | Relevance to Qgen |
|---|---|---|---|
| `multer` 2.0.2 | direct dependency | Four separate high-severity DoS advisories (unclosed streams, deeply nested fields, incomplete cleanup, resource exhaustion) | Directly reachable — `/ask` and `/context/upload` both accept multipart input; compounds M-02 |
| `undici` (pinned `6.20.1`) | via `cheerio` | Multiple high/moderate: memory exhaustion, request/response smuggling, CRLF and header injection, insufficiently random values | The `pnpm.overrides` entry **pins** `undici` to `6.20.1`, below the patched `>=6.27.0` — this override actively holds the package on a vulnerable version |
| `path-to-regexp` | via `express` | High-severity ReDoS | Reachable on every request through route matching |
| `lodash` | transitive | Code injection via `_.template`, prototype pollution | Depends on call sites in dependents |
| `ws` | transitive | Memory-exhaustion DoS, uninitialised memory disclosure | Reachable via puppeteer's DevTools transport |
| `basic-ftp`, `extract-zip` | via `puppeteer-core` | Critical path traversal; symlink traversal | Primarily install-time, but present in the runtime tree |
| `js-yaml`, `qs`, `fast-uri`, `ip-address`, `file-type` | transitive | Quadratic-complexity DoS, host confusion, parser loops | Parsing paths reachable from request handling |

The `pnpm.overrides` block in `package.json` was clearly added as a remediation measure, but several entries have since fallen behind the advisories they were meant to address — `undici` is pinned exactly, not floored, and now holds a vulnerable version in place.

**Impact.** Several of these are directly reachable from unauthenticated or single-key-authenticated request paths and provide low-effort denial of service. Combined with the complete absence of rate limiting (H-05), a single client can trigger them at will.

**Remediation.** Change the `undici` override from `"6.20.1"` to `">=6.27.0"` and re-audit; upgrade `multer` to the latest 2.x patch release; run `pnpm update --latest` on the transitive tree and re-check. Add `pnpm audit --prod --audit-level=high` as a required CI step (see L-06) and enable Dependabot or Renovate on the repository so advisories surface without manual review.

---

## 5. Medium-Severity Findings

### M-01 — Context traversal in `POST /ask` leaks file contents through model output

**CWE-22.** [app.service.ts:383](src/app.service.ts#L383), [app.service.ts:245](src/app.service.ts#L245), [rag.service.ts:38](src/rag/rag.service.ts#L38).

`AskDto.context` is validated only by `@IsString()` and flows into `join(process.cwd(), 'data', 'contexts', contextName)` in three separate services. Supplying a relative traversal directs `getMarkdownFiles` at an arbitrary directory; every `.md` file found there is concatenated into the LLM system prompt, and the model's response — which quotes and summarises that material — is returned to the caller. This is a lower-bandwidth variant of C-01 that survives any fix applied only to the context controller, which is why the sanitisation helper must be applied at the service layer. Constrain `context` with `@Matches(/^[a-z0-9-]+$/)` and route all three sites through `safeJoin`.

### M-02 — Unbounded upload buffering before size validation

**CWE-770.** [app.controller.ts:163](src/app.controller.ts#L163), [context.controller.ts:225](src/context/context.controller.ts#L225).

`FileInterceptor('file')` is used with no options, so Multer applies **no `limits`** and buffers the entire upload into process memory. The 5 MB ceilings in `MarkdownFileValidator` and `ParseFilePipe` are pipe-stage checks that run only *after* the body is fully buffered. A single multi-gigabyte upload — or several concurrent ones, since nothing rate-limits them — exhausts heap and terminates the process. Pass explicit limits at the interceptor: `FileInterceptor('file', { limits: { fileSize: 5 * 1024 * 1024, files: 1, fields: 10 } })`, and enforce a matching `client_max_body_size` at the reverse proxy.

### M-03 — Error text and stack traces published to a public ntfy.sh topic

**CWE-532: Insertion of Sensitive Information into Log File.** [notification-logger.service.ts:34-48](src/notification-logger.service.ts#L34-L48).

`NotificationLoggerService` overrides `error()` globally and POSTs the message plus all `optionalParams` — which is where NestJS passes stack traces — to `https://ntfy.sh/${NTFY_TOPIC}`. ntfy.sh topics are **public by default**: anyone who knows or guesses the topic name can subscribe and receive the stream. The default topic is the highly guessable `qgen-errors`, and `.env.template` ships that value. Error messages throughout the codebase interpolate raw `error.message`, which for filesystem failures includes absolute paths and for HTTP client failures can include request context.

Use an authenticated, access-controlled notification channel; failing that, generate a long random topic name and treat it as a secret. Send only an error class and a correlation identifier, never the message body or stack, and keep full detail in local logs.

### M-04 — No security headers, no CORS policy, Swagger exposed unauthenticated

**CWE-16: Configuration; CWE-1188: Insecure Default Initialization.** [main.ts:39-45](src/main.ts#L39-L45).

`bootstrap()` registers no `helmet` middleware, so responses carry no `Strict-Transport-Security`, `X-Content-Type-Options`, `X-Frame-Options`, `Content-Security-Policy`, or `Referrer-Policy`. This matters concretely because `GET /` returns `Content-Type: text/html` — the application does serve a browser-rendered page.

`SwaggerModule.setup('api', …)` runs unconditionally in every environment. Swagger's routes are registered as raw Express middleware and are therefore **not** subject to `APP_GUARD`, so `/api` is publicly readable: it discloses the complete route inventory, parameter schemas, and example payloads, which is a direct aid to exploiting C-01, C-02 and H-01. `persistAuthorization: true` additionally causes the UI to store submitted API keys in browser `localStorage`.

CORS is not configured; Nest's default is disabled, which is the correct posture, but it is implicit rather than declared and would be lost to a careless `enableCors()`.

Add `helmet()`. Gate Swagger on `NODE_ENV !== 'production'` or place it behind authentication. Declare an explicit CORS policy with a named origin allowlist. Remove `persistAuthorization` for any deployment where the UI is reachable.

### M-05 — Non-constant-time comparison of API keys and master key

**CWE-208: Observable Timing Discrepancy.** [api-key.service.ts:86](src/auth/api-key.service.ts#L86), [auth.controller.ts:62](src/auth/auth.controller.ts#L62).

Both `apiKeys.find((k) => k.key === key)` and `createApiKeyDto.masterKey !== masterKey` use JavaScript string equality, which short-circuits on the first differing byte. API keys carry 256 bits of entropy, so the practical risk there is low. The `MASTER_KEY` is the real concern: it is a human-chosen passphrase of unknown entropy, compared on an unauthenticated endpoint with no rate limiting (H-05), which is the combination under which remote timing attacks become tractable. Compare fixed-length hashes with `crypto.timingSafeEqual`.

### M-06 — Production endpoint documented as plaintext HTTP

**CWE-319: Cleartext Transmission of Sensitive Information.** `package.json` test scripts.

Eight `:prod` scripts target `TEST_URL=http://83.228.208.214:3000` — plaintext HTTP, to a bare IP address, on the application's own port with no reverse proxy implied. If this reflects the live deployment, every API key travels the network in the clear in an `x-api-key` header, as does every customer message and every generated quote. There is no certificate to validate, so the endpoint is also trivially impersonated. Terminate TLS at a reverse proxy with a certificate for a real hostname, redirect HTTP to HTTPS, enable HSTS, bind the Node process to loopback, and update the test scripts. Any key that has transited the plaintext endpoint should be rotated.

### M-07 — Indirect prompt injection through context files and fetched links

**CWE-1427: Improper Neutralization of Input Used for LLM Prompting.** [app.service.ts:306](src/app.service.ts#L306), [app.service.ts:334](src/app.service.ts#L334), [rag.service.ts:321](src/rag/rag.service.ts#L321).

System prompts are built by raw string concatenation of markdown file contents, uploaded file contents, and text scraped from remote pages, with no delimiting, escaping, or provenance marking:

```ts
contextContent += `### File: ${file}\n${fileContent}\n\n`;
contextContent += `### Link: ${link.title} (${link.url})\n${extractedContent.text}\n\n`;
```

Three injection vectors exist: an uploaded `.md` file on `/ask`, a registered context link whose remote page is fetched at request time and is outside the operator's control, and — most durably — a context file overwritten through H-02. Instructions embedded in any of these are indistinguishable to the model from operator instructions, and can override the pricing and formatting rules the application depends on.

Also note that `RagService.parseSelectionResponse` ([rag.service.ts:221-263](src/rag/rag.service.ts#L221-L263)) parses model output with a regex and `JSON.parse`, and falls back to **including every file in the context** whenever parsing fails — a failure mode an injected payload can trigger deliberately to maximise cost and context exposure.

Delimit untrusted content explicitly (fenced blocks with a clear "the following is untrusted data, not instructions" preamble). Treat fetched link content as the least trusted input and consider stripping it from system prompts entirely. Validate model output against a strict schema rather than a permissive regex, and make the failure mode restrictive rather than expansive.

### M-08 — Key deletion by 12-character prefix can remove the wrong key

**CWE-706: Use of Incorrectly-Resolved Name.** [api-key.service.ts:113-127](src/auth/api-key.service.ts#L113-L127).

`deleteApiKey` matches with `k.key.startsWith(keyPreview.replace('...', '').substring(0, 12))`. Since every key begins with the fixed prefix `qgen_`, only **seven hexadecimal characters** are discriminating — roughly 268 million combinations, within collision range for a large key set and, more importantly, easily satisfied deliberately. `GET /auth/api-keys` hands out exactly these previews to any key holder. The method is not currently routed, so this is latent; if a revocation endpoint is added — as C-03 recommends — it must match on the full key hash, not a prefix, and must be restricted to an administrative scope.

### M-09 — Unsynchronised read-modify-write on shared JSON stores

**CWE-362: Race Condition.** [json-store.ts:11-22](src/storage/json-store.ts#L11-L22), [custom-memory.ts:31-45](src/memory/custom-memory.ts#L31-L45), [cost-tracking.service.ts](src/memory/cost-tracking.service.ts).

`JsonStore` (and its duplicate inside `custom-memory.ts`) implements read-all, mutate, write-all with no locking and non-atomic writes. `appendLog`, `giveFeedback`, `saveContext` and the cost tracker all follow this pattern on files shared across all concurrent requests. Under concurrency, log entries and conversation turns are silently lost; on an interrupted write, the file is truncated and the reader's `catch` returns an empty object, discarding all history without error. The same weakness in `api-key.service.ts` is escalated to High as H-06 because that file is the credential store; here the consequence is data loss and unreliable audit and billing records. Apply atomic write-and-rename plus a per-file mutex, or move this state to SQLite, which would resolve the concurrency, durability, and query concerns together.

---

## 6. Low-Severity Findings

### L-01 — Secret files are world-readable

**CWE-732: Incorrect Permission Assignment for Critical Resource.** On the audited host, `.env` (containing four provider keys, `GITHUB_API_TOKEN`, `MASTER_KEY`, and an Ethereum `PRIVATE_KEY`) and `data/api-keys.json` are both mode `0644` — readable by every local account. Set `0600` on both, own them by the service account, and prefer environment injection or a secret manager over on-disk files in production.

### L-02 — `forbidNonWhitelisted` disabled on the global validation pipe

**CWE-915: Improperly Controlled Modification of Dynamically-Determined Object Attributes.** [main.ts:19](src/main.ts#L19). `whitelist: true` strips unknown properties but `forbidNonWhitelisted: false` accepts the request silently rather than rejecting it. Unexpected fields therefore fail open and are never surfaced. Set it to `true` so malformed or probing requests are rejected explicitly.

### L-03 — Internal error messages propagated to clients

**CWE-209: Information Exposure Through an Error Message.** Every `catch` block in `ContextController` re-throws `error.message` inside an `HttpException` (for example [context.controller.ts:75-78](src/context/context.controller.ts#L75-L78)). Node filesystem errors embed absolute paths, disclosing the deployment directory layout and confirming traversal success or failure. Return a generic message with a correlation identifier and log the detail server-side.

### L-04 — `Math.random()` used in request identifiers

**CWE-338: Use of Cryptographically Weak PRNG.** [app.service.ts:389](src/app.service.ts#L389). `requestId` is built from `Date.now()` plus `Math.random().toString(36)`. These identifiers address log entries that `POST /feedback` can mutate, so predictability has a minor authorisation consequence on top of collision risk. Use `randomUUID()`, which is already imported in the same file. (The deprecated `String.prototype.substr` is also used here.)

### L-05 — Core business logic excluded from version control

**Process weakness.** `.gitignore` lists `src/app.service.ts` — the 1,033-line service that orchestrates prompt construction, model fallback, cost tracking, and logging. It is therefore absent from code review, CI, and the audit trail; no reviewer sees changes to it, and a fresh clone cannot build. This is likely an accident (a stale entry from a template) but it is a significant governance gap given the file's role. Remove the entry and commit the file after confirming it contains no secrets.

### L-06 — CI performs no dependency scanning and no frozen-lockfile install

**Supply-chain weakness.** `.github/workflows/test.yml` runs `pnpm install` (not `--frozen-lockfile`), permitting silent lockfile drift in CI, and pins `pnpm/action-setup@v2` with pnpm 8 while the project declares `packageManager: pnpm@10.23.0`. No `pnpm audit`, SAST, or secret-scanning step exists — which is why H-08's 49 production advisories accumulated unnoticed. Add `--frozen-lockfile`, align the pnpm version, and add `pnpm audit --prod --audit-level=high` plus secret scanning as required checks.

### L-07 — Weak default master key in the environment template

**CWE-1188.** `.env.template` ships `MASTER_KEY='super-secure-passphrase'`. Templates become production configuration more often than anyone intends, and this value is guessable on the first attempt against the unauthenticated, unthrottled endpoint described in H-05. Replace it with an explicit placeholder such as `MASTER_KEY='CHANGE_ME_openssl_rand_hex_32'`, and have the application refuse to start if the configured value matches the template or falls below a minimum length.

### L-08 — Indefinite retention of customer conversation content

**Privacy / GDPR.** `data/chat-history.json` accumulates every user and assistant turn with no expiry, no per-user deletion path, and no encryption at rest ([custom-memory.ts:67-89](src/memory/custom-memory.ts#L67-L89)). `data/logs.json` similarly retains messages and caller-supplied `userId` values until a weekly report truncates it — into `reports/`, which is itself retained indefinitely. For a French construction-quoting product these records contain customer names, addresses, and project details, engaging GDPR obligations around retention limitation, erasure, and access control. Define a retention period and enforce it, implement per-subject erasure, encrypt these files at rest, and document the retention policy.

---

## 7. Consolidated Remediation Plan

### Phase 1 — Immediate (before any further exposure)

| # | Action | Findings |
|---|---|---|
| 1 | Restrict network access to the service until Phase 2 lands, or take it offline if it is currently internet-facing | C-01, C-02 |
| 2 | Rotate every credential in `.env` — Anthropic, OpenAI, Mistral, DeepSeek, GitHub token, ntfy token, `MASTER_KEY` — and move any funds controlled by the Ethereum `PRIVATE_KEY` | C-01, C-03 |
| 3 | Revoke and reissue all existing API keys | C-03, M-06 |
| 4 | Add a `safeJoin` containment helper and apply it to all nine user-controlled path constructions in `context.service.ts`, `app.service.ts`, and `rag.service.ts` | C-01, C-02, H-02, M-01 |
| 5 | Validate `:name`, `:filename`, `context`, and upload filenames against strict character classes | C-01, C-02, H-02, M-01 |
| 6 | `chmod 600 .env data/api-keys.json` | L-01 |

### Phase 2 — Short term (1–2 weeks)

| # | Action | Findings |
|---|---|---|
| 7 | Hash API keys at rest; compare with `timingSafeEqual`; stop rewriting the store on the auth path | C-03, H-06, M-05 |
| 8 | Add `@nestjs/throttler` globally, with a strict limit and lockout on `/auth/api-keys` | H-05 |
| 9 | Add an SSRF allowlist with post-redirect re-validation; remove `--no-sandbox` | H-01 |
| 10 | Add `helmet()`; gate Swagger on non-production; declare an explicit CORS policy | M-04 |
| 11 | Add Multer `limits` at both `FileInterceptor` call sites | M-02 |
| 12 | Terminate TLS in front of the service; bind Node to loopback; update `:prod` test scripts | M-06 |
| 13 | Fix the `undici` override (`>=6.27.0`), upgrade `multer`, re-run `pnpm audit --prod` | H-08 |
| 14 | Convert the GitHub GraphQL query to variables, or delete `SubsService` if it is dead code | H-07 |

### Phase 3 — Medium term (1–2 months)

| # | Action | Findings |
|---|---|---|
| 15 | Add owner and scope fields to `ApiKey`; enforce scopes with a guard; filter `/logs` by owner; verify ownership in `giveFeedback` | H-03 |
| 16 | Bind sessions to their creating identity; reject foreign `sessionId` values; add `@IsUUID()` | H-04 |
| 17 | Enforce per-key spend and request quotas in `CostTracker`; bound the model fallback loop | H-05 |
| 18 | Replace JSON-file storage with SQLite (atomic, transactional, concurrent-safe) | H-06, M-09 |
| 19 | Delimit untrusted content in prompts; validate model output against a schema with restrictive failure modes | M-07 |
| 20 | Move error notifications to an authenticated channel; send only class and correlation id | M-03 |
| 21 | Set `forbidNonWhitelisted: true`; return generic client errors; use `randomUUID()` for request ids | L-02, L-03, L-04 |
| 22 | Un-ignore `src/app.service.ts`; add `--frozen-lockfile`, dependency scanning and secret scanning to CI | L-05, L-06 |
| 23 | Define and enforce data retention; implement per-subject erasure; encrypt stored transcripts | L-08 |

---

## 8. Positive Observations

Several aspects of the implementation are sound and worth preserving:

- **Key generation is cryptographically correct.** `qgen_` plus 32 bytes from `crypto.randomBytes` yields 256 bits of entropy — the keys themselves are not guessable. The weakness is entirely in storage and comparison, not generation.
- **Secure-by-default guard registration.** `ApiKeyGuard` is registered as an `APP_GUARD`, so routes are protected unless explicitly marked `@Public()`. Only two routes are public and both are deliberate. This is the right default direction, and it means adding scopes (H-03) extends an existing structure rather than retrofitting one.
- **No secrets in version control.** Git history contains no `.env` or `data/api-keys.json` commits; `.gitignore` covers the secret and state files correctly.
- **DTO validation is present and used.** A global `ValidationPipe` with `whitelist: true` is configured, and several DTOs carry appropriate constraints. The gaps identified above are places where existing DTOs were not bound (H-02) or where route parameters bypassed them (C-01) — not an absence of the pattern.
- **Dependency overrides show active maintenance.** The `pnpm.overrides` block demonstrates that transitive advisories have been addressed before; the block simply needs refreshing (H-08).
- **Reasonable test coverage.** Unit specs accompany most services, alongside e2e acceptance and guardrail suites across all four model providers.
- **Cost tracking is thorough.** Per-model rates are documented and dated, and cost is attributed per request — a solid foundation on which to build the spend limits recommended in H-05.

---

## 9. Methodology and Limitations

**Performed.** Manual review of all 58 TypeScript source files; data-flow tracing from every HTTP entry point to filesystem, network, and LLM sinks; dependency advisory analysis via `pnpm audit` (full tree and production-only); review of CI configuration, environment template, `.gitignore`, and git history for committed secrets; and empirical verification of two exploit preconditions in an isolated local harness — Express 5 route-parameter URL-decoding behaviour (confirming C-01/C-02) and `class-validator`'s `@IsUrl()` acceptance of internal IP literals (confirming H-01).

**Not performed.** No dynamic testing against a running instance and no exploitation of live infrastructure; findings marked "verified empirically" were confirmed against the project's own installed library versions in isolation, not against a deployed Qgen. No review of host, container, or network configuration, which was outside the repository. No adversarial LLM red-teaming of the prompt-injection surface (M-07) beyond structural analysis. No formal CVSS scoring by a certified assessor — the vectors given are the author's estimates for prioritisation only.

**Assumptions.** The `TEST_URL` in `package.json` (`http://83.228.208.214:3000`) is taken to reflect a real production deployment; if it is a disposable staging host, M-06 should be re-rated against the actual production configuration. `data/contexts/batappli` is committed to the repository and assumed present in every deployment, which is what satisfies the existence precondition for C-01.

---

*Prepared 4 September 2026. Findings reflect commit `6a57364` on branch `63-improve-instructions-for-surface-calculation`, including uncommitted working-tree modifications present at the time of review.*