Security Audit — Qgen (Batappli Quote Generator API)

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

1.2 Findings index

2. System Overview

2.1 Attack surface

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.

3. Critical Findings

C-01 — Path traversal enables arbitrary file read

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

// 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:

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

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:

// 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

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:

// 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

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:

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) 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). 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

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:

// 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:

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

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), 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);
• 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), 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

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

// 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) 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

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). 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 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). 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)

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

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

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

// 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:

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

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:

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, app.service.ts:245, rag.service.ts:38.

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, context.controller.ts:225.

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.

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.

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, auth.controller.ts:62.

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, app.service.ts:334, rag.service.ts:321.

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:

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) 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.

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, custom-memory.ts:31-45, 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. 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). 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. 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). 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)

Phase 2 — Short term (1–2 weeks)

Phase 3 — Medium term (1–2 months)

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.