🤖 Code Audit: 23 potential issue(s) found

[asmit25805]

Code Audit Report

All findings are reviewed for confidence before posting.
Please verify each finding before acting on it.

Repository: superloglabs/superlog
Findings: 23 issue(s) found — 🔴 1 critical · 🟠 11 high · 🟡 8 medium · 🔵 3 low

---

1. 🐛 Infinite loop on Windows platforms

Field	Details
Severity	🔴 Critical
Type	Bug
File	packages/db/drizzle.config.ts
Location	function findRepoRoot(start: string)
Confidence	95%

Problem:
The loop condition checks while (dir !== "/"), which only works on Unix-like systems. On Windows, the root path is like "C:" and will never equal "/", causing dirname to return the same path repeatedly and resulting in an infinite loop.

Suggested Fix:
Replace the loop termination check with a platform‑agnostic condition, e.g., while (dir !== path.parse(dir).root) { ... } or break when dirname(dir) === dir.

---

2. 🔒 Bearer token printed to stdout

Field	Details
Severity	🟠 High
Type	Security
File	scripts/mint-mcp-token.ts
Location	console.log(plaintext);
Confidence	95%

Problem:
The script outputs the generated plaintext bearer token to standard output. If the console output is logged, captured, or stored in shell history, the token can be leaked, allowing unauthorized access to protected resources.

Suggested Fix:
Avoid printing the token to stdout. Instead, store it securely (e.g., in a secret manager) or output only a reference/identifier. If printing is required for debugging, ensure the environment is isolated and that logs are not persisted.

---

3. 🔒 Insecure gRPC credentials used for exporter

Field	Details
Severity	🟠 High
Type	Security
File	scripts/smoke-grpc.ts
Location	new OTLPTraceExporter({ credentials: credentials.createInsecure() })
Confidence	94%

Problem:
The exporter is configured with credentials.createInsecure(), which disables TLS. If the OTLP_GRPC_URL points to a remote or production endpoint, data will be transmitted without encryption and could be intercepted or tampered with.

Suggested Fix:
Replace credentials.createInsecure() with credentials.createSsl() (or appropriate secure credentials) and ensure the endpoint uses TLS. Provide the necessary root certificates if needed.

---

4. 🐛 Top-level await may cause syntax error in CommonJS environments

Field	Details
Severity	🟠 High
Type	Bug
File	scripts/smoke-proto.ts
Location	await provider.shutdown();
Confidence	95%

Problem:
The script uses await at the top level. Top‑level await is only supported in ES modules. When the TypeScript is compiled to CommonJS (the default for many Node projects), this line will cause a syntax error and the script will fail to start.

Suggested Fix:
Wrap the asynchronous logic in an async IIFE, e.g., (async () => { /* existing code */ })();, or configure the project to emit ES modules (set "type": "module" in package.json or use tsc --module esnext).

---

5. 🔒 API key exposed via command‑line arguments

Field	Details
Severity	🟠 High
Type	Security
File	scripts/worktree-ensure-telemetry.ts
Location	spawnSync( ... )
Confidence	95%

Problem:
The freshly minted API key (minted.plaintext) is passed to the child process as a command‑line argument. On many operating systems the full command line is visible to other users (e.g., via ps), leaking the secret and allowing an attacker to impersonate the project.

Suggested Fix:
Pass the API key to the child process through an environment variable or via stdin, and ensure the child script reads it from there. For example, add API_KEY to the env object and modify the called script to read process.env.API_KEY instead of a CLI flag.

---

6. 🔒 Unvalidated .env file path can lead to arbitrary file read

Field	Details
Severity	🟠 High
Type	Security
File	apps/api/tracing.ts
Location	if (process.env.SUPERLOG_ENV_FILE) { loadDotenv({ path: process.env.SUPERLOG_ENV_FILE, override: true }); }
Confidence	92%

Problem:
The code loads an environment file from a path supplied directly via the SUPERLOG_ENV_FILE environment variable without any validation or sanitisation. An attacker who can influence this variable could cause the application to read arbitrary files on the filesystem, potentially exposing secrets or causing denial‑of‑service.

Suggested Fix:
Validate the SUPERLOG_ENV_FILE value against an allow‑list of expected filenames or directories, and reject or sanitize any path that contains traversal characters (e.g., "..", absolute paths).

---

7. 🐛 OTEL_SDK_DISABLED is set after OTel modules are imported

Field	Details
Severity	🟠 High
Type	Bug
File	apps/proxy/tracing.ts
Location	process.env.OTEL_SDK_DISABLED assignment (around line 30)
Confidence	95%

Problem:
The code sets process.env.OTEL_SDK_DISABLED = "true" to disable the OpenTelemetry SDK when telemetry is not enabled, but the import statements for OTel packages are evaluated before any top‑level code runs. Because imports are hoisted, the SDK reads the environment variable during module initialization, so setting it later has no effect. Telemetry may still be sent in non‑production environments, contradicting the intended behavior.

Suggested Fix:
Move the assignment of process.env.OTEL_SDK_DISABLED (and any other env‑based configuration) to a location that runs before the OTel imports, e.g., place the environment‑variable logic in a separate module that is required first, or use dynamic import() after the variable is set.

---

8. 🐛 Potential NaN port value causing Vite server startup failure

Field	Details
Severity	🟠 High
Type	Bug
File	apps/web/vite.config.ts
Location	port: Number(env.PORT ?? env.WEB_PORT ?? 5173)
Confidence	92%

Problem:
The code converts the environment variable PORT or WEB_PORT to a number using Number(). If the variable is undefined, empty, or contains a non‑numeric string, Number() returns NaN. Vite expects a valid integer port; passing NaN can cause the server to crash or fail to start, leading to runtime errors.

Suggested Fix:
Parse the port value with parseInt and validate the result, falling back to a safe default when conversion fails. Example:

let rawPort = env.PORT ?? env.WEB_PORT;
let port = rawPort ? parseInt(rawPort, 10) : 5173;
if (Number.isNaN(port) || port <= 0) {
  port = 5173; // fallback default
}

Then use port in the server configuration.

---

9. 🔒 Unvalidated command-line arguments passed to privileged collector binary

Field	Details
Severity	🟠 High
Type	Security
File	infra/collector/entrypoint.go
Location	main() / collectorArgs()
Confidence	92%

Problem:
The program forwards any arguments received (os.Args[1:]) directly to the collector binary via syscall.Exec without validation or sanitization. If the collector binary runs with elevated privileges, an attacker could supply malicious flags or arguments to influence its behavior, potentially leading to privilege escalation or unintended actions.

Suggested Fix:
Implement strict validation or whitelisting of allowed arguments before passing them to the collector binary. Reject or sanitize any unexpected flags, and consider constructing the argument list explicitly rather than forwarding user-provided args.

---

10. 🐛 Missing validation for empty DATABASE_URL

Field	Details
Severity	🟠 High
Type	Bug
File	packages/db/drizzle.config.ts
Location	dbCredentials: { url: readDatabaseUrl() }
Confidence	92%

Problem:
If none of the lookup methods provide a URL, readDatabaseUrl returns an empty string. Passing an empty URL to Drizzle's configuration will cause runtime errors during migration or connection attempts, making the failure harder to diagnose.

Suggested Fix:
After calling readDatabaseUrl, check if the result is a non‑empty string and throw a clear error if not, e.g., const url = readDatabaseUrl(); if (!url) { throw new Error('DATABASE_URL not found'); }.

---

11. 🔒 Plaintext API key logged to console

Field	Details
Severity	🟠 High
Type	Security
File	scripts/demo/bootstrap-acme.ts
Location	main -> console.log(JSON.stringify(... plaintext ...))
Confidence	96%

Problem:
The script generates an API key and includes the plaintext value in the JSON output printed to stdout. This can expose the secret key to logs, terminals, or any process that captures stdout, leading to credential leakage.

Suggested Fix:
Remove the plaintext field from the output. Store the plaintext key securely (e.g., display it once to the user and then discard) and avoid logging it in production scripts.

---

12. 🐛 NaN services count when non-numeric value is supplied

Field	Details
Severity	🟠 High
Type	Bug
File	scripts/demo/seed-rich-telemetry.ts
Location	parseArgs function (services field)
Confidence	92%

Problem:
The services field is calculated with Math.min(8, Math.max(1, Number(map.get("services") ?? 4))). If the user provides a non‑numeric value, Number() returns NaN, causing Math.max(1, NaN) to produce NaN and consequently Math.min(8, NaN) also returns NaN. This propagates to later logic that expects a numeric count, potentially causing runtime errors or empty loops.

Suggested Fix:
Validate the parsed value before using it, e.g., const svc = Number(map.get("services") ?? 4); if (isNaN(svc) || svc < 1) throw new Error('invalid services count'); const services = Math.min(8, Math.max(1, Math.floor(svc)));

---

13. 🔒 Potential credential leakage in error output

Field	Details
Severity	🟡 Medium
Type	Security
File	scripts/ensure-database.ts
Location	if (!dbName) { ... console.error("no database name in url:", targetUrl);
Confidence	92%

Problem:
When the script cannot extract a database name from the provided URL, it logs the full URL to stderr. If the URL contains embedded credentials (e.g., postgres://user:password@host/db), those credentials are exposed in logs, which may be collected or viewed by unauthorized parties.

Suggested Fix:
Avoid printing the full URL in error messages. Instead, log only the problematic part (e.g., the pathname) or a generic error. Example: console.error("no database name in url");

---

14. 🐛 Incorrect default URL scheme for gRPC OTLP exporter

Field	Details
Severity	🟡 Medium
Type	Bug
File	scripts/smoke-grpc.ts
Location	const url = process.env.OTLP_GRPC_URL ?? "http://localhost:4317";
Confidence	88%

Problem:
The OTLPTraceExporter expects a gRPC endpoint in the form host:port or a grpc:// scheme. Using http://localhost:4317 may cause the exporter to fail to connect because the HTTP scheme is not valid for gRPC transport.

Suggested Fix:
Change the default to a proper gRPC address, e.g., const url = process.env.OTLP_GRPC_URL ?? "localhost:4317"; or grpc://localhost:4317.

---

15. 🔒 Insecure default OTLP endpoint uses HTTP

Field	Details
Severity	🟡 Medium
Type	Security
File	scripts/smoke-proto.ts
Location	const url = process.env.OTLP_URL ?? "http://localhost:4000/v1/traces";
Confidence	88%

Problem:
If the OTLP_URL environment variable is not set, the exporter falls back to an HTTP endpoint. Transmitting trace data over plain HTTP can expose sensitive information to network eavesdropping.

Suggested Fix:
Change the default to use HTTPS, e.g., "https://localhost:4000/v1/traces", and ensure the collector supports TLS. Optionally enforce that OTLP_URL must be provided.

---

16. 🔒 Potential SQL injection in ClickHouse query

Field	Details
Severity	🟡 Medium
Type	Security
File	scripts/worktree-ensure-telemetry.ts
Location	chTraceCount function
Confidence	90%

Problem:
The ClickHouse query string interpolates projectId directly into the SQL statement without any escaping. If projectId ever contains malicious characters (e.g., a quote), an attacker could manipulate the query and retrieve or corrupt data.

Suggested Fix:
Use parameterized queries or properly escape the identifier. For ClickHouse, you can send the query as a prepared statement or at minimum sanitize projectId with a whitelist/escaping function before embedding it.

---

17. 🐛 Potential loss of PORT environment variable when PORTLESS_URL is set

Field	Details
Severity	🟡 Medium
Type	Bug
File	apps/api/tracing.ts
Location	const portlessPort = process.env.PORTLESS_URL ? process.env.PORT : undefined; ... if (portlessPort) process.env.PORT = portlessPort;
Confidence	85%

Problem:
When PORTLESS_URL is defined but PORT is undefined, portlessPort becomes undefined. The subsequent assignment process.env.PORT = portlessPort deletes the PORT variable, which may break downstream code that expects PORT to be set.

Suggested Fix:
Store the original PORT value only if it is defined, e.g., const portlessPort = process.env.PORTLESS_URL ? process.env.PORT ?? null : undefined; and only reassign when the stored value is a non‑null string.

---

18. 🔒 Unvalidated OTLP endpoint can lead to SSRF

Field	Details
Severity	🟡 Medium
Type	Security
File	apps/proxy/tracing.ts
Location	OTLP exporter URL construction (around lines 70‑80)
Confidence	88%

Problem:
The exporter URLs are built directly from the OTEL_EXPORTER_OTLP_ENDPOINT environment variable without any validation. If an attacker can influence this variable, they could cause the application to send telemetry data to an arbitrary server, potentially exfiltrating sensitive information or performing a server‑side request forgery (SSRF) attack.

Suggested Fix:
Validate the OTEL_EXPORTER_OTLP_ENDPOINT value against an allow‑list of trusted hosts or enforce a strict URL schema (e.g., only https:// URLs). Reject or sanitize malformed values before constructing the exporter URLs.

---

19. ⚡ Sequential database operations cause unnecessary latency

Field	Details
Severity	🟡 Medium
Type	Performance
File	scripts/demo/bootstrap-acme.ts
Location	main -> sequential db queries (user, org, membership, project, apiKey, automation)
Confidence	88%

Problem:
The script performs many independent DB queries one after another (e.g., fetching/creating user, org, membership, project, API key, automation settings). Each await blocks the next operation, increasing total execution time, especially over high-latency connections.

Suggested Fix:
Parallelize independent operations using Promise.all where possible (e.g., create org and user concurrently, then handle memberships). Also consider batching related inserts/updates to reduce round trips.

---

20. 🐛 Potential division by zero when points is zero

Field	Details
Severity	🟡 Medium
Type	Bug
File	scripts/demo/seed-rich-telemetry.ts
Location	buildMetrics function (stepMs calculation)
Confidence	85%

Problem:
stepMs is computed as (opts.minutes * 60_000) / opts.points. If opts.points is zero (e.g., user passes --points 0), this results in a division by zero, yielding Infinity and later producing invalid timestamps for metric data points.

Suggested Fix:
Add a guard to ensure opts.points is a positive integer, e.g., if (opts.points <= 0) throw new Error('points must be > 0'); before the calculation.

---

21. 💡 Missing error handling for provider shutdown

Field	Details
Severity	🔵 Low
Type	Suggestion
File	scripts/smoke-proto.ts
Location	await provider.shutdown();
Confidence	82%

Problem:
The call to provider.shutdown() is awaited but any rejection is not caught, which could hide shutdown failures and make debugging harder.

Suggested Fix:
Wrap the shutdown call in a try/catch block or attach a .catch handler, e.g., await provider.shutdown().catch(err => console.error('Shutdown error:', err));.

---

22. ⚡ Unnecessary import of OpenTelemetry SDK when telemetry is disabled

Field	Details
Severity	🔵 Low
Type	Performance
File	apps/api/tracing.ts
Location	import statements for OTel SDK modules
Confidence	81%

Problem:
The file imports the OpenTelemetry SDK modules at the top level regardless of the telemetryEnabled flag. Even when telemetry is disabled, these modules are loaded, incurring import overhead and possible side‑effects.

Suggested Fix:
Move the OpenTelemetry imports inside the if (telemetryEnabled) { ... } block or use dynamic import() to load them lazily only when telemetry is enabled.

---

23. 💡 Improve .env.local parsing robustness

Field	Details
Severity	🔵 Low
Type	Suggestion
File	packages/db/drizzle.config.ts
Location	readDatabaseUrl() regex extraction
Confidence	85%

Problem:
The current regex only matches lines exactly starting with DATABASE_URL= and captures everything after the equals sign, which may miss values surrounded by quotes or preceded by whitespace/comments.

Suggested Fix:
Use a more tolerant parser, such as splitting the file into lines and trimming each line, or leverage a dotenv library to correctly parse quoted values and ignore comments.

---

About this report

This report was generated using Llama 3.3 70B.
Only findings with ≥80% confidence are included.
False positives are possible — use your own judgment.