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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,12 @@ Once a report is received through the GitHub Security Advisory form, we commit t
## GitHub Security Advisories

Maintainers: Please ensure that **GitHub Security Advisories** are enabled for this repository to allow researchers to submit reports privately.

## Logging and Secret Redaction

Server logs are configured to redact Stellar secret keys to prevent accidental leakage. The backend uses `pino` with the following protections:

- Structured field redaction for paths: `*.secretKey`, `*.privateKey`, `*.seed`.
- Regex-based redaction for any string matching `^S[0-9A-Z]{55}$` (Stellar secret seed format).

If you discover logs containing secret material, please follow the private reporting process above.
2 changes: 1 addition & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"ioredis": "^5.3.2",
"jsonwebtoken": "^9.0.2",
"p-limit": "^4.0.0",
"prom-client": "^15.1.3",

"swagger-ui-express": "^5.0.1",
"ws": "^8.20.0",
"zod": "^4.3.6"
Expand Down
19 changes: 19 additions & 0 deletions backend/src/logger.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { redactObject } from "./logger";

describe("logger redaction", () => {
test("redacts stellar secret keys in objects and strings", () => {
const secret = "S" + "A".repeat(55);
const obj = {
nested: { secretKey: secret, other: "ok" },
token: secret,
arr: [secret, { privateKey: secret }],
} as const;

const redacted = redactObject(obj as any);

expect(redacted.nested.secretKey).toBe("[REDACTED]");
expect(redacted.token).toBe("[REDACTED]");
expect(redacted.arr[0]).toBe("[REDACTED]");
expect(redacted.arr[1].privateKey).toBe("[REDACTED]");
});
Comment on lines +3 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an integration-style assertion against actual logger output.

Current coverage only tests redactObject; it does not verify emitted log lines are redacted, which is part of the acceptance criteria. Also add an explicit seed field assertion to cover all required paths.

Suggested test expansion
 import { redactObject } from "./logger";
+import { logger } from "./logger";

 describe("logger redaction", () => {
   test("redacts stellar secret keys in objects and strings", () => {
@@
-    const obj = {
+    const obj = {
       nested: { secretKey: secret, other: "ok" },
+      wallet: { seed: secret },
       token: secret,
       arr: [secret, { privateKey: secret }],
     } as const;
@@
     expect(redacted.nested.secretKey).toBe("[REDACTED]");
+    expect(redacted.wallet.seed).toBe("[REDACTED]");
     expect(redacted.token).toBe("[REDACTED]");
     expect(redacted.arr[0]).toBe("[REDACTED]");
     expect(redacted.arr[1].privateKey).toBe("[REDACTED]");
   });
+
+  test("does not emit raw stellar secrets in logger output", () => {
+    const secret = "S" + "A".repeat(55);
+    const spy = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
+
+    logger.info({ secretKey: secret, token: secret }, "test log");
+
+    const written = spy.mock.calls.map(([chunk]) => String(chunk)).join("");
+    expect(written).not.toContain(secret);
+    expect(written).toContain("[REDACTED]");
+    spy.mockRestore();
+  });
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/logger.test.ts` around lines 3 - 18, Tests only exercise
redactObject directly but don't assert the logger actually emits redacted lines
nor check the seed field; update the "logger redaction" test to call the real
logging path (use the module's logger or logging function that wraps
redactObject) and capture emitted log output (e.g., spy or capture
console/process logs) then assert the logged message string contains
"[REDACTED]" for each secret occurrence; also add an explicit assertion that a
"seed" field (e.g., obj.seed or similar key) is redacted in both the object
result from redactObject and in the emitted log output. Ensure you reference
redactObject and the exported logger/logging function used by the code under
test when adding the integration assertion.

});
41 changes: 41 additions & 0 deletions backend/src/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import pino from "pino";

const STELLAR_SECRET_REGEX = /^S[0-9A-Z]{55}$/;

function redactValue(value: unknown): unknown {
if (typeof value === "string" && STELLAR_SECRET_REGEX.test(value)) {
return "[REDACTED]";
}
return value;
}

function redactObject(obj: any): any {
if (obj == null) return obj;
if (Array.isArray(obj)) return obj.map(redactObject);
if (typeof obj === "object") {
const out: Record<string, any> = {};
for (const [k, v] of Object.entries(obj)) {
if (k === "secretKey" || k === "privateKey" || k === "seed") {
out[k] = "[REDACTED]";
} else {
out[k] = redactObject(v);
}
}
return out;
}
return redactValue(obj);
}

const logger = pino({
level: process.env.LOG_LEVEL || "info",
// keep path-based redaction for structured fields
redact: { paths: ["*.secretKey", "*.privateKey", "*.seed"], censor: "[REDACTED]" },
// ensure values (strings) that match Stellar secret pattern are redacted anywhere
formatters: {
log(obj: Record<string, any>) {
return redactObject(obj);
},
},
});

export { logger, redactObject, STELLAR_SECRET_REGEX };
13 changes: 3 additions & 10 deletions backend/src/middleware/requestLogger.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Request, Response, NextFunction } from "express";
import crypto from "crypto";
import { logger } from "../logger";

declare global {
namespace Express {
Expand Down Expand Up @@ -30,16 +31,8 @@ export function requestLogger(req: Request, res: Response, next: NextFunction) {
duration: `${duration}ms`,
};

// ✅ STEP 4: Make logs readable in development
if (process.env.NODE_ENV === "production") {
// Machine-readable (for logging systems)
console.log(JSON.stringify(logEntry));
} else {
// Human-readable (for local dev)
console.log(
`${req.method} ${req.originalUrl} ${res.statusCode} - ${duration}ms | id=${requestId}`
);
}
// ✅ STEP 4: Use structured logger with redaction
logger.info(logEntry, `${req.method} ${req.originalUrl} ${res.statusCode} - ${duration}ms | id=${requestId}`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid embedding raw URL data in the log message string.

Line 35 duplicates req.originalUrl into free-form text, which can leak sensitive query values. Keep message static and rely on structured fields (and preferably log req.path instead of full originalUrl when possible).

Safer logging pattern
-    logger.info(logEntry, `${req.method} ${req.originalUrl} ${res.statusCode} - ${duration}ms | id=${requestId}`);
+    logger.info(logEntry, "request_completed");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
logger.info(logEntry, `${req.method} ${req.originalUrl} ${res.statusCode} - ${duration}ms | id=${requestId}`);
logger.info(logEntry, "request_completed");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/middleware/requestLogger.ts` at line 35, The current logger.info
call in requestLogger.ts embeds req.originalUrl into the free-form message which
can leak sensitive query data; change the call that references
logger.info(logEntry, `${req.method} ${req.originalUrl} ${res.statusCode} -
${duration}ms | id=${requestId}`) so the message text is static (e.g., "Request
completed") and move dynamic values into structured fields instead, using
req.path (not req.originalUrl) for the URL, and include method, res.statusCode,
duration and requestId as separate properties on the log entry so sensitive
query params are not written into the free-form message.

});

// ✅ STEP 5: Continue request
Expand Down