diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 34122c2b4..a92cae563 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -6,7 +6,7 @@ ## 2024-07-07 - Unsanitized Directory Input Paths API Validation **Vulnerability:** The API logic allowed user-controlled local data directory paths (`cacheRoot` and `tempRoot`) to be directly used without mitigating cross-platform path traversal vulnerabilities. **Learning:** Checking for '..' sequences in untrusted paths fails to parse cross-platform separators reliably for untrusted inputs (e.g., Windows backslashes on POSIX). Relying solely on `os.sep` or `os.altsep` is inadequate because absolute paths can bypass restrictions if not resolved correctly, or if `os.altsep` is None. -**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g., `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. +**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g. `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. ## 2024-05-20 - Python Path Traversal Mitigation bypass **Vulnerability:** Path traversal detection in Python backend APIs relied solely on checking the input path string or basic parsed parts which might not adequately catch sequences like `..` when intermixed with different path separators. @@ -28,3 +28,10 @@ **Vulnerability:** The Rust backend (`apps/desktop/src-tauri/src/main.rs`) did not enforce a maximum URL length limit when processing YouTube URLs via `import_youtube_url`. While the frontend enforced `MAX_YOUTUBE_URL_LENGTH = 2000` via the input element, this could be bypassed by an attacker sending requests directly to the Tauri backend API, potentially causing a Denial of Service (DoS) due to unbounded URL parsing and regex matching. **Learning:** Input validation must occur at the entry point of untrusted data on the backend, even if it is also validated on the frontend. Relying solely on frontend validation for constraints like string length can expose the backend to resource exhaustion vulnerabilities. **Prevention:** Always enforce constraints like maximum length, format validation, and sanitization at the earliest possible point on the backend, typically at the API boundary, regardless of frontend safeguards. + +## 2026-09-05 - CSV Formula Injection C0 Control Prefix Bypass +**Vulnerability:** CSV formula-injection mitigation was incomplete when a cell began with a C0 control character (`\x00`-`\x1F`) that could be interpreted differently by downstream spreadsheet or parser implementations before a formula token. +**Learning:** NUL is only one member of the parser-disagreement boundary. Security policy must not depend on every downstream consumer preserving leading control bytes exactly, and executable regressions must include non-whitespace controls such as ESC as well as NUL. +**Prevention:** In `escapeCsvField`, treat any leading C0 control after permitted whitespace/BOM/NBSP as dangerous, prefix the entire original field before structural CSV quoting, and retain regressions for NUL-only, repeated NUL, whitespace+control, ESC-prefixed formula-shaped values, and full-width formula operators. Keep the lint exception scoped only to the intentional control-character regular expression. + +**Repair lineage:** The inherited `test_supply_chain_policy.py` formatting prerequisite remains owned by PR #1176 at `a7b0030a3a6cc6296a19ba3f8eaf595d470d05bd`; this security branch integrates that exact commit as ancestry instead of copying a competing edit. diff --git a/apps/desktop/src/lib/export.test.ts b/apps/desktop/src/lib/export.test.ts index 265e983d4..e0e97ddd6 100644 --- a/apps/desktop/src/lib/export.test.ts +++ b/apps/desktop/src/lib/export.test.ts @@ -67,6 +67,26 @@ describe("export sanitization", () => { expect(escapeCsvField("\t+SUM(A1)")).toBe("'\t+SUM(A1)"); expect(escapeCsvField("\n-100")).toBe("\"'\n-100\""); expect(escapeCsvField("\r@cmd")).toBe("\"'\r@cmd\""); + + // Prevent bypasses using NUL bytes, including a NUL-only cell. + expect(escapeCsvField("\x00=1+2")).toBe("'\x00=1+2"); + expect(escapeCsvField(" \x00@cmd")).toBe("' \x00@cmd"); + expect(escapeCsvField("\x00\x00=1+2")).toBe("'\x00\x00=1+2"); + expect(escapeCsvField(" \x00\x00@cmd")).toBe("' \x00\x00@cmd"); + expect(escapeCsvField("\x00")).toBe("'\x00"); + + // Spreadsheet/parser disagreement is not limited to NUL: fail closed on any leading C0 control. + expect(escapeCsvField("\x1B+SUM(A1)")).toBe("'\x1B+SUM(A1)"); + expect(escapeCsvField(" \x07@cmd")).toBe("' \x07@cmd"); + expect(escapeCsvField("\x1B")).toBe("'\x1B"); + }); + + it("preserves the full-width operator regression contract from PR #941", () => { + expect(escapeCsvField("=1+2")).toBe("'=1+2"); + expect(escapeCsvField("+SUM(A1)")).toBe("'+SUM(A1)"); + expect(escapeCsvField("-100")).toBe("'-100"); + expect(escapeCsvField("@cmd")).toBe("'@cmd"); + expect(escapeCsvField(" \uFEFF=SUM(A1)")).toBe("' \uFEFF=SUM(A1)"); }); it("handles combined scenarios: formula injection with structural characters", () => { diff --git a/apps/desktop/src/lib/export.ts b/apps/desktop/src/lib/export.ts index 3d4493b1d..9d45ed6fc 100644 --- a/apps/desktop/src/lib/export.ts +++ b/apps/desktop/src/lib/export.ts @@ -11,7 +11,7 @@ import { // Security notes: // 1. Filename sanitization to prevent directory traversal or invalid characters. -// 2. CSV formula injection prevention (fields starting with =, +, -, @ must be prefixed with a single quote). +// 2. CSV formula injection prevention (dangerous ASCII/full-width formula or control initiators receive a single-quote prefix). /** Documented. */ export function sanitizeFilename(title: string): string { @@ -22,8 +22,9 @@ export function sanitizeFilename(title: string): string { /** Documented. */ export function escapeCsvField(value: string): string { let escapedValue = value; - // Prevent CSV formula injection by prefixing problematic leading characters with a single quote - if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/.test(value)) { + // Spreadsheet/parser disagreement can make a leading C0 control security-significant even when it precedes an operator. + // eslint-disable-next-line no-control-regex + if (/^[\s\uFEFF\xA0]*[\x00-\x1F=+\-@\uFF1D\uFF0B\uFF0D\uFF20]/.test(value)) { escapedValue = `'${value}`; } // Enclose in double quotes if there's a comma, newline, or double quote diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 1d8224c5a..6a0853944 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -1275,9 +1275,7 @@ def test_workflow_concurrency_cancels_only_superseded_pr_heads() -> None: workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") assert "concurrency:" in workflow, workflow_name assert "cancel-in-progress: false" in workflow, workflow_name - assert "contents: read" in workflow or "permissions: read-all" in workflow, ( - workflow_name - ) + assert "contents: read" in workflow or "permissions: read-all" in workflow, workflow_name assert "pull_request:" not in (workflows_dir / "release.yml").read_text(encoding="utf-8")