diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..377ae3d --- /dev/null +++ b/.env.example @@ -0,0 +1,85 @@ +# Copy to .env and adjust. Every value below is the built-in default, so an +# unset variable behaves exactly like the line shown. +# +# cp .env.example .env +# +# .env is gitignored. Never commit real secrets. + +# --- Required in production -------------------------------------------------- + +# Flask/CSRF signing key. The built-in development default is publicly known, so +# the app REFUSES TO START with it when APP_ENV=production. +# python -c "import secrets; print(secrets.token_hex(32))" +SECRET_KEY=dev-secret-key-change-in-production + +# The single canonical production signal. Setting it to "production" enables the +# SECRET_KEY fail-fast, the Secure session cookie, and the rate-limit topology +# guard. No other spelling is accepted -- two names would let a deployment +# satisfy one gate and silently miss another. +# APP_ENV=production + +# --- Deployment topology (see README "Deployment topology and rate limiting") -- + +# Worker count. Single source of truth: gunicorn reads it natively and every +# documented start command passes --workers "$WEB_CONCURRENCY". Required under +# APP_ENV=production. +WEB_CONCURRENCY=1 + +# Replica count. Invisible from inside the process, so it is declared here and +# must mirror render.yaml's numInstances. Required under APP_ENV=production. +APP_REPLICAS=1 + +# Rate-limit counter storage. memory:// counters are process-local, so the +# effective limit is multiplied by workers x replicas. Anything above 1x1 +# requires a shared backend (and requirements-redis.txt installed). +RATELIMIT_STORAGE_URI=memory:// + +# Trust X-Forwarded-* from exactly one proxy hop. Enable ONLY when the app sits +# behind a proxy you control; otherwise clients can forge their rate-limit bucket. +TRUST_PROXY=0 + +# --- Limits ------------------------------------------------------------------ + +MAX_UPLOAD_SIZE=10485760 +PREVIEW_ROW_LIMIT=25 +FLATTEN_MAX_DEPTH=10 + +# XLSX-only export budget, in cells (rows x columns). Derived from the +# measurement in docs/export-budget-v1.2.md. CSV/TSV are streamed and uncapped. +# 0 disables the guard. +MAX_EXPORT_CELLS=250000 + +# --- API fetch --------------------------------------------------------------- + +API_FETCH_TIMEOUT=30 +API_FETCH_MAX_RESPONSE=10485760 + +# Ports the API-fetch feature may connect to. Empty disables the check. +API_ALLOWED_PORTS=80,443,8443 + +# DNS admission control. API_DNS_TIMEOUT bounds how long a REQUEST waits, not how +# long the lookup runs -- getaddrinfo exposes no timeout and cannot be cancelled. +# API_DNS_MAX_WORKERS bounds concurrency, which is the actual starvation fix. +API_DNS_TIMEOUT=3 +API_DNS_MAX_WORKERS=4 +API_DNS_ADMISSION_TIMEOUT=1 + +# --- Rate limits ------------------------------------------------------------- + +RATE_LIMIT_DEFAULT=120/minute +RATE_LIMIT_PROCESS=30/minute +RATE_LIMIT_EXPORT=60/minute + +# --- Misc -------------------------------------------------------------------- + +# Cache lifetime for static assets. Safe because asset URLs carry ?v=APP_VERSION. +STATIC_MAX_AGE=86400 + +# Responses below this size are not worth compressing. +GZIP_MIN_SIZE=1024 + +# Set to 0 to omit the version from /health. +HEALTH_REVEAL_VERSION=1 + +# Flask debug mode. Never enable in production. +FLASK_DEBUG=0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..2480cd9 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,88 @@ +name: CI + +# Every commit is covered exactly once: a branch commit by its pull_request run, +# a main commit by its push run. +# +# `push: ["**"]` alongside `pull_request` ran BOTH on every push to a PR branch. +# The concurrency group below collapsed that pair, but collapsing means one of +# the two is cancelled -- and GitHub attaches a cancelled check run to the head +# commit, where it is not a success. So the PR reported `mergeable_state: +# unstable` on every push even with CI fully green, which is indistinguishable +# at a glance from a real failure. Not generating the duplicate beats cancelling +# it. +# +# The trade-off: a branch with no pull request open gets no CI. That is the +# intended shape -- the checks exist to gate the merge, and opening the PR is +# what starts them. +on: + push: + branches: ["main"] + pull_request: + +# Nothing here touches the GitHub API beyond checkout, so the token needs no +# write scope; an explicit block also stops it inheriting wider repo defaults. +permissions: + contents: read + +# Supersede a branch's own earlier run when a new commit lands on it: pushing +# three times in a minute should not leave three runs competing for runners to +# report on commits nobody is waiting for any more. +# +# This cancels only runs for SUPERSEDED commits, never the current head's -- +# the `on:` block above is what guarantees one run per commit, so there is no +# same-SHA pair left to collapse. A cancelled check run lands on the old commit, +# which is why this no longer costs the head commit its green state. +# +# The branch name alone is not unique across repositories: two pull requests +# opened from different forks on a branch both named `feature` would share a +# group, and cancel-in-progress would have them kill each other's checks. Keying +# on the source repo keeps forks apart. head_ref is the source branch on a +# pull_request and empty on a push; ref_name is the branch without the +# refs/heads/ prefix (github.ref would keep it). +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.head.repo.full_name || github.repository }}-${{ github.head_ref || github.ref_name }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # Nothing after this step talks to GitHub, so leaving the token in + # .git/config only widens what a compromised dependency can reach. + persist-credentials: false + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-dev.txt + + - name: Lint + run: ruff check . + + - name: Format check + run: ruff format --check . + + - uses: actions/setup-node@v4 + with: + node-version: "22" + + - name: Tests + run: python -m pytest tests/ -v + + - name: Client-side export assertions (F1) + run: node tests/js/test_export_sanitize.mjs + + - name: Client-side render/cap assertions (P4/P5/P13) + run: node tests/js/test_render_caps.mjs + + - name: Client-side feature assertions (Phase 4) + run: node tests/js/test_features.mjs + + - name: Audit runtime dependencies + run: pip-audit -r requirements.txt diff --git a/.gitignore b/.gitignore index 6e098ba..8b96fd9 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,9 @@ ENV/ .DS_Store Thumbs.db +# Node (test assertions only; no build step) +node_modules/ + # Project specific *.log .env diff --git a/AGENTS.md b/AGENTS.md index e7c4914..12287c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,15 +18,15 @@ Contract for AI coding agents (Claude Code, Cursor, GitHub Copilot Workspace, Ai | Item | Value | |-------------------|----------------------------------------------------------------| | Language | Python 3.11+ (Render targets 3.14.5) | -| Web framework | Flask 3.0.0 (app factory) | +| Web framework | Flask 3.1.3 (app factory) | | Security libs | Flask-WTF 1.2.1 (CSRF), Flask-Limiter 3.5.0 (rate limit) | -| HTTP client | requests 2.31.0 | -| Excel export | openpyxl 3.1.2 | -| WSGI server | gunicorn 21.2.0 — invoked as `gunicorn "app:create_app()"` | -| Test runner | pytest 7.4.4 | +| HTTP client | requests 2.33.0 | +| Excel export | openpyxl 3.1.5 | +| WSGI server | gunicorn 22.0.0 — `gunicorn "app:create_app()" --workers "$WEB_CONCURRENCY" --timeout 60` | +| Test runner | pytest 9.0.3 (`requirements-dev.txt`) + ruff, coverage, pip-audit | | Frontend | HTML + external CSS/JS (no framework, no build) | | Deployment | Render (free tier) via `render.yaml`; also docs for Docker/Nginx | -| App version | `Config.APP_VERSION = '1.1.0'` (returned by `/health`) | +| App version | `Config.APP_VERSION = '1.2.0'` (returned by `/health`) | ## Setup @@ -37,7 +37,7 @@ python -m venv .venv # macOS/Linux source .venv/bin/activate -pip install -r requirements.txt +pip install -r requirements-dev.txt python app.py # http://localhost:5000 # enable debug @@ -56,43 +56,52 @@ python -m pytest tests/ -v ### Backend -- **`app.py`** — `create_app(config_class=Config)` factory. Initializes CSRF + Limiter, registers `apply_security_headers` as an `after_request` middleware, registers the `routes.bp` Blueprint. Module-level `app = create_app()` exists for tooling that expects it, but production uses the factory directly. -- **`config.py`** — `Config` class; every setting reads from `os.environ.get(...)` with a default. Includes `SECRET_KEY`, `MAX_CONTENT_LENGTH`, `PREVIEW_ROW_LIMIT`, `API_FETCH_TIMEOUT`, `API_FETCH_MAX_RESPONSE`, `FLATTEN_MAX_DEPTH`, `RATELIMIT_*`, `APP_VERSION`, `DEBUG`. -- **`extensions.py`** — Bare `CSRFProtect()` and `Limiter(key_func=get_remote_address)` instances, bound by `app.py` via `init_app`. Importing this module never has side effects on the Flask app — that's the point. +- **`app.py`** — `create_app(config_class=Config)` factory. Asserts the production SECRET_KEY, runs `check_rate_limit_topology`, optionally installs `ProxyFix` (`TRUST_PROXY=1`), initializes CSRF + Limiter, registers `apply_security_headers` and `compress_response` as `after_request` middleware plus the JSON 413/500/404 handlers, and registers the `routes.bp` Blueprint. Module-level `app = create_app()` exists for tooling that expects it, but production uses the factory directly. +- **`config.py`** — `Config` class plus `is_production()`, `env_int()` and `env_int_set()`. Every setting reads from the environment with a default; integers report which variable was mistyped instead of raising a bare `ValueError`. Includes `SECRET_KEY`, `MAX_CONTENT_LENGTH`, `PREVIEW_ROW_LIMIT`, `API_FETCH_*`, `API_DNS_*`, `API_ALLOWED_PORTS`, `FLATTEN_MAX_DEPTH`, `MAX_EXPORT_CELLS`, `RATELIMIT_*`, `WEB_CONCURRENCY`, `APP_REPLICAS`, `TRUST_PROXY`, `SESSION_COOKIE_*`, `SEND_FILE_MAX_AGE_DEFAULT`, `GZIP_MIN_SIZE`, `HEALTH_REVEAL_VERSION`, `APP_VERSION`, `DEBUG`. See `.env.example`. +- **`extensions.py`** — Bare `CSRFProtect()` and `Limiter(key_func=client_ip_key)` instances, bound by `app.py` via `init_app`. `client_ip_key` reads `request.remote_addr` and never the raw `X-Forwarded-For`, so the bucket is whatever ProxyFix decided rather than something a client can assert. Importing this module never has side effects on the Flask app — that's the point. - **`security.py`** - - `validate_url(url)` — returns `(is_valid, error_or_none)`. Rejects non-http(s) schemes, missing hostname, non-resolvable hostnames, and any resolved IP where `not ip.is_global or ip.is_multicast`. - - `apply_security_headers(response)` — sets CSP (strict, `script-src 'self'`), `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`. CSP allows Google Fonts (style/font) and `data:` images. + - `validate_url(url)` — returns `(is_valid, error_or_none)`. Rejects non-http(s) schemes, missing hostname, ports outside `API_ALLOWED_PORTS`, non-resolvable hostnames, and any resolved IP where `not ip.is_global or ip.is_multicast`. + - `resolve_hostname(hostname)` / `get_resolver_pool()` — DNS on a shared fixed-size pool with admission control. Bounds the caller's wait and concurrency; **not** the lookup itself, and **not** teardown. + - `apply_security_headers(response)` — sets CSP (strict, `script-src 'self'`, plus `object-src 'none'`, `base-uri 'self'`, `frame-ancestors 'none'`, `form-action 'self'`, `upgrade-insecure-requests`), `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`, `Permissions-Policy`, COOP/CORP, HSTS on secure requests only, and `Cache-Control: no-store` on the data and health endpoints. CSP allows Google Fonts (style/font) and `data:` images. - **`helpers.py`** - `flatten_for_csv(data, parent_key='', sep='.', _depth=0, max_depth=10)` — depth-capped recursion; deep nesting is JSON-stringified instead of stack-overflowing. - - `extract_table_data(json_data)` — heuristic: list-of-dicts → use directly; dict with array value → that array; nested dicts → recurse; otherwise single-row. + - `extract_table_data(json_data, _depth=0, max_depth=10)` — heuristic: list-of-dicts → use directly; dict with array value → that array; nested dicts → recurse (depth-capped); otherwise single-row. + - `flatten_rows(rows, max_depth=10)` — one pass returning `(flattened_rows, sorted_columns)`; replaces flatten-then-`get_all_columns`. + - `sanitize_cell(value)` / `serialize_cell_value(value)` / `is_formula_trigger(value)` — spreadsheet formula-injection defenses. **CSV/TSV/XLSX only.** + - `preview_truncate(row)` — capped *copy* of a preview row; never mutates the source. + - `format_size(num_bytes)` — byte count in the largest non-zero unit. Shared utilities live here, **never in `app.py`**: `create_app()` imports `bp` from `routes.py`, so importing up creates a cycle. - `parse_jsonl(text)` — line-by-line JSON; raises `ValueError` with the offending line number. - - `find_candidate_arrays(json_data, prefix='', candidates=None)` — discovers every array-of-objects with `{path, length, sample_keys}` metadata so the frontend can prompt the user. - `extract_by_path(json_data, path)` — dot-notation navigation; `'(root)'` is the sentinel for top-level lists. - `get_all_columns(data)` — sorted union of keys. - **`routes.py`** — Blueprint `bp`. Routes: - `GET /` → `templates/index.html`. - - `GET /health` → `{"status": "ok", "version": APP_VERSION}`. - - `POST /process` → rate-limited (default `RATE_LIMIT_PROCESS=30/min`). Reads `input_method` (`file`/`paste`/`api`), `data_format` (`json`/`jsonl`), optional `json_path`. Returns `{success, columns, preview, total_rows, csv_data, csv_columns}` **or** `{needs_selection: true, candidates: [...]}` when multiple arrays found and no `json_path` selected. - - `POST /export-csv` → rate-limited (default `RATE_LIMIT_EXPORT=60/min`). Server-side CSV fallback. - - `POST /export-xlsx` → rate-limited. Server-side Excel via openpyxl. + - `GET /health` → `{"status": "ok", "version": APP_VERSION}` (version omitted when `HEALTH_REVEAL_VERSION=0`). + - `GET /health/live` → process liveness; checks nothing, so a dependency outage cannot cause a restart loop. + - `GET /health/ready` → readiness; 200 or 503 with a `checks` map. + - `POST /process` → rate-limited (default `RATE_LIMIT_PROCESS=30/min`). Reads `input_method` (`file`/`paste`/`api`), `data_format` (`json`/`jsonl`), optional `json_path`. Returns `{success, columns, preview, preview_limit, total_rows, total_cells, max_export_cells, csv_data, csv_columns}` **or** `{needs_selection: true, raw_json: ...}` when no `json_path` was selected, so the frontend can render the JSON tree picker. The body is built by `_load_input()` and `_select_table_data()`; keep `process_json` itself thin. + - `POST /export-csv` → rate-limited (default `RATE_LIMIT_EXPORT=60/min`). Generator-streamed and **uncapped**. + - `POST /export-xlsx` → rate-limited. Server-side Excel via openpyxl, capped by `MAX_EXPORT_CELLS` (400 when exceeded, never truncated). Diskless: no OS temp files. ### Frontend - **`templates/index.html`** — pure structure. CSRF meta tag at the top (``). References `style.css` and `app.js` via `url_for('static', ...)`. No inline JS/CSS (CSP would block it). -- **`static/css/style.css`** — `:root` (dark, default) + `:root.light` overrides. All component styles, sort indicators, modal, export dropdown, theme toggle. -- **`static/js/app.js`** — reads CSRF token from meta tag; attaches it to FormData and `X-CSRFToken`. Handles tab switching, drag-drop, JSON/JSONL toggle, auth method visibility, client-side sort, **client-side** CSV/TSV download, **server-side** Excel via `/export-xlsx`, theme persistence in `localStorage`, and the path-selection modal triggered by `needs_selection: true`. +- **`static/css/style.css`** — `:root` (dark, default) + `:root.light` overrides. All component styles, sort indicators, modals, export dropdown, table toolbar (filter / load-more / column visibility), theme toggle. +- **`static/js/app.js`** — reads CSRF token from meta tag; attaches it to FormData and `X-CSRFToken`. Handles tab switching, drag-drop, JSON/JSONL toggle, auth method visibility, client-side sort/filter/pagination/column visibility, **client-side** CSV, TSV, JSONL and Markdown downloads, **server-side** Excel via `/export-xlsx`, theme persistence in `localStorage`, the lazily-built JSON tree picker triggered by `needs_selection: true` (with `#path=` deep links), and the About modal. No `alert()`. ### Tests - **`tests/conftest.py`** — provides `app` (with `TESTING=True`, `WTF_CSRF_ENABLED=False`) and `client` fixtures. - **`tests/test_helpers.py`** — pure-function tests for the data-processing layer. - **`tests/test_security.py`** — `validate_url` with mocked `socket.getaddrinfo`; verifies private-IP/loopback/link-local rejection. -- **`tests/test_routes.py`** — integration tests for every route, including security headers, JSONL, path selection, Excel export. +- **`tests/test_routes.py`** — integration tests for every route, including security headers, JSONL, path selection, exports, formula injection, log hygiene, config gates and the topology guard. +- **`tests/js/*.mjs`** — Node assertions that load the real `static/js/app.js` in a stubbed DOM (`dom_stub.mjs`). No build step, no dependencies. Run them with `make test-js`; CI runs them too. ### Deploy / Config -- **`render.yaml`** — Render Blueprint. `startCommand: gunicorn "app:create_app()" --bind 0.0.0.0:$PORT`. `SECRET_KEY` is `generateValue: true` so Render auto-fills it. -- **`requirements.txt`** — exact-pinned. Don't loosen. +- **`render.yaml`** — Render Blueprint. The start command derives `--workers` from `$WEB_CONCURRENCY` and sets `--timeout 60`; `APP_ENV=production`, `WEB_CONCURRENCY` and `APP_REPLICAS` are declared; `SECRET_KEY` is `generateValue: true`; `autoDeployTrigger: checksPass` gates deploys on CI. +- **`requirements.txt`** — exact-pinned runtime deps. Don't loosen. `requirements-dev.txt` holds test tooling; `requirements-redis.txt` holds the optional Redis client for shared rate-limit storage. +- **`.github/workflows/ci.yml`** — ruff check, ruff format --check, pytest, the Node assertions, and `pip-audit`. +- **`Makefile` / `.env.example`** — developer entry points and every environment variable with its default. ## Conventions @@ -103,7 +112,11 @@ python -m pytest tests/ -v - Apply `@limiter.limit(lambda: current_app.config.get('RATE_LIMIT_...'))` on any new mutating route. - Read config via `current_app.config['KEY']`, not by re-importing `Config` at request time. - For new auth methods on the API tab: add a select option in `index.html`, a `data-auth="..."` fieldset, the JS visibility branch in `app.js`, and the conditional in `routes.py`. -- For new exports: add the entry to the export dropdown, the handler in `app.js`, optionally a server route in `routes.py`. Pin any new dependency. +- For new exports: add the entry to the export dropdown, the handler in `app.js`, optionally a server route in `routes.py`. Pin any new dependency. **Decide the sanitization policy explicitly.** Spreadsheet-compatible formats need one of two defenses, not both: + - **Quote-prefix via `sanitize_cell`** for formats with no type channel (CSV, TSV). The prefix is the only place a value can be marked as text. + - **Pin the cell type** for formats that have one. `/export-xlsx` sets `data_type='s'` on every cell, so openpyxl writes a string cell and Excel never evaluates it; prefixing on top would put a stray `'` in the data. + + Non-spreadsheet formats (JSONL, Markdown) get neither — a prefix there corrupts values while protecting nothing. - Add or update tests under `tests/` for any backend behavior change. Class-style grouping (`class TestXxx:`) is the existing pattern. - Preserve the CSP. If new third-party CSS/fonts are needed, edit `apply_security_headers` deliberately. @@ -115,21 +128,33 @@ python -m pytest tests/ -v - Don't unbound the streamed download — `API_FETCH_MAX_RESPONSE` (10 MB default) is enforced inside the chunk loop. - Don't lower `FLATTEN_MAX_DEPTH` recursion guard without checking known payload shapes. - Don't log full exceptions to the response — the routes intentionally return generic messages and `logger.exception(...)` for server logs. -- Don't use the dev `SECRET_KEY` in production. Render generates one via `render.yaml`; for other deployments set the env var. +- Don't use the dev `SECRET_KEY` in production. Render generates one via `render.yaml`; for other deployments set the env var. `create_app` refuses to start on it when `APP_ENV=production`. +- Don't infer production from `not DEBUG`, and don't add a second spelling of `APP_ENV` — `config.is_production()` is the only signal. +- Don't log the URL, the exception, or any request field on the API-fetch failure path — a token can ride in the query string, fragment, userinfo *or* path. +- Don't describe the DNS resolver teardown as bounded. `API_DNS_TIMEOUT` bounds the caller's wait; `getaddrinfo` cannot be cancelled. +- Don't write payloads to disk. openpyxl's `write_only` mode and a rolled-over `SpooledTemporaryFile` both do. +- Don't truncate an oversized export — refuse it. A partial spreadsheet is worse than none. +- Don't hardcode a bare `--workers N` in a start command; derive it from `$WEB_CONCURRENCY` so the declared and running counts cannot drift. +- Don't relax the CSP, and don't reintroduce `alert()` — the About dialog is an in-page modal. ## Verification Checklist (before reporting a task done) -- [ ] `python -m pytest tests/ -v` passes. -- [ ] `python app.py` starts cleanly; `GET /` renders; `GET /health` returns the current `APP_VERSION`. +- [ ] `python -m pytest tests/ -v` passes (the passing command is the criterion, not a count). +- [ ] `ruff check .` and `ruff format --check .` exit 0. +- [ ] The Node assertions pass: `make test-js`. +- [ ] `pip-audit -r requirements.txt` reports 0 vulnerabilities. +- [ ] `python app.py` starts cleanly; `GET /` renders; `GET /health` returns the current `APP_VERSION`; `/health/live` and `/health/ready` respond. - [ ] CSP headers still present (`curl -sI http://localhost:5000/ | findstr /i security` or browser DevTools). - [ ] CSRF still required on POSTs (a POST without the token returns 400 from Flask-WTF). - [ ] All three input methods (file / paste / API), both formats (JSON / JSONL), and all four auth methods still work end to end. -- [ ] Multi-array JSON triggers the path-selector modal; selecting a path returns rows. -- [ ] CSV (client-side), TSV (client-side), and XLSX (server-side) all download with full row counts, not just 25. -- [ ] SSRF guard blocks `http://127.0.0.1`, `http://localhost`, `http://169.254.169.254`, and similar private IPs. +- [ ] Multi-array JSON triggers the JSON tree picker; selecting a path returns rows; `#path=...` pre-selects one. +- [ ] CSV, TSV, JSONL and Markdown (client-side) and XLSX (server-side) all download with full row counts, not just the preview. +- [ ] A cell value of `=SUM(A1)` opens as inert text in CSV and XLSX, and survives verbatim in JSONL and Markdown. +- [ ] SSRF guard blocks `http://127.0.0.1`, `http://localhost`, `http://169.254.169.254`, `http://2130706433`, `http://0x7f000001`, `http://[::ffff:7f00:1]`, and any port outside `API_ALLOWED_PORTS`. - [ ] Rate limit kicks in at the configured threshold (manual: rapid-fire `/process`). -- [ ] No new file writes, DB calls, payload logging, or inline JS/CSS were introduced. -- [ ] Any new dependency is **exact-pinned** in `requirements.txt`. +- [ ] No new file writes, DB calls, payload logging, inline JS/CSS, or CSP relaxations were introduced. +- [ ] Any new dependency is **exact-pinned** in `requirements.txt` (or `requirements-dev.txt` / `requirements-redis.txt`). +- [ ] `.env.example`, `README.md` and `CHANGELOG.md` cover any new environment variable. ## When in Doubt diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..64ec424 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,140 @@ +# Changelog + +All notable changes to this project are documented here. + +The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and +this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [1.2.0] - 2026-08-21 — "Hardening & Performance" + +Implements `docs/roadmap-v1.2.md`, closing every finding in +`docs/security-review-v1.2.md` (F1–F17) and `docs/performance-review-v1.2.md` +(P1–P13). + +No response key changed name, type or meaning; `/process` only gained keys. + +### Security + +- **CSV/Excel formula injection (F1, Critical).** Values beginning with + `=`, `+`, `-`, `@`, tab, CR or LF were written verbatim into every export. + CSV/TSV now prefix them with a single quote; XLSX pins the cell's `data_type` + to a string, since openpyxl otherwise serializes a leading `=` as a formula. + Applies to all four export paths, headers included. JSONL and Markdown exports + are deliberately exempt — see *Added*. +- **Dependency CVEs (F2, High).** Flask 3.1.3, requests 2.33.0, gunicorn 22.0.0 + (CVE-2024-1135 request smuggling), openpyxl 3.1.5. + `pip-audit -r requirements.txt` goes from 7 vulnerabilities to 0. +- **Rate limiting behind a proxy (F12, High).** Opt-in `TRUST_PROXY=1` installs + ProxyFix with exactly one trusted hop, so users stop sharing a single bucket. + Off by default; forwarded headers are ignored unless enabled. +- **Credential leakage into logs (F3/F9).** The API-fetch failure log is now a + fixed string. `requests`' exception text carries the full URL, and the query + string, fragment, userinfo *and* path can each hold a token. +- **Outbound header allowlist (F4).** The client supplies the header *name* for + API-key auth; names are stripped and lowercased before an explicit allowlist + check, so `Host`, `Proxy-Authorization` and `CoNnEcTiOn` no longer pass. +- **Security headers (F5).** Added HSTS (secure requests only), + `Permissions-Policy`, `Cross-Origin-Opener-Policy` and + `Cross-Origin-Resource-Policy`; CSP gained `object-src 'none'`, + `base-uri 'self'`, `frame-ancestors 'none'`, `form-action 'self'` and + `upgrade-insecure-requests`. +- **SSRF: bounded DNS and a port allowlist (F6).** Lookups run on a shared, + fixed-size pool with admission control, so a slow nameserver no longer holds a + request thread for the length of the lookup: the caller returns on + `API_DNS_TIMEOUT`, and once the pool is saturated further requests are refused + on `API_DNS_ADMISSION_TIMEOUT` instead of queueing. The lookup itself is not + bounded — `getaddrinfo` exposes no timeout and cannot be cancelled, so the + pool thread stays occupied until the platform resolver returns. + `reset_resolver_pool()` shuts the pool down with `wait=False` and returns + immediately, so it does not block on that thread; interpreter exit still can. `API_ALLOWED_PORTS` defaults to `80,443,8443`. +- **SECRET_KEY fail-fast (F7).** `APP_ENV=production` with the publicly known + dev key (or an empty one) refuses to start. Integer settings now report which + variable was mistyped. +- **Recursion-depth DoS (F8).** `extract_table_data` gained the depth cap + `flatten_for_csv` already had, and `RecursionError` anywhere in the pipeline + returns 400 `JSON nesting too deep` instead of 500. +- **JSON error handlers (F10).** 413, 500 and 404 return JSON, so the client's + `response.json()` no longer throws on an HTML body. +- **`Cache-Control: no-store` (F11)** on all data-bearing and health responses. +- **Upload validation (F13).** Non-`.json`/`.jsonl` filenames and clearly wrong + content types are rejected server-side. +- **Cookie hardening (F16).** `HttpOnly`, `SameSite=Lax`, and `Secure` tied to + `APP_ENV=production`. + +### Performance + +- **gzip compression (P1).** ~40 lines of middleware, no new dependency. Skips + streamed, bodyless, already-encoded and non-text responses. +- **Diskless, memory-bounded exports (P3).** CSV/TSV are generator-streamed and + uncapped. XLSX keeps a normal-mode workbook (no OS temp files) plus + `MAX_EXPORT_CELLS`, measured rather than guessed — see + `docs/export-budget-v1.2.md`. +- **Lazy tree picker (P4)** and **client render caps (P5)**: children build on + first toggle; nested objects stop at 20 keys, primitive arrays at 20 items, + strings at 500 characters. +- **Non-mutating preview truncation (P2.2/P5).** The preview is a capped copy; + `csv_data` and every export keep full fidelity. +- **Static asset caching (P6)** for a day, behind `?v=APP_VERSION` URLs. +- **Memory trims (P8/P12).** One-pass flatten-and-collect-columns, and the + API path decodes its `bytearray` without an intermediate copy. +- **gunicorn `--timeout 60` (P9)** everywhere, above `API_FETCH_TIMEOUT`. +- **Chunked Blob for client exports (P13).** + +### Added + +- `/health/live` and `/health/ready` alongside the unchanged `/health`. +- **Load more / Load all** over the full dataset, with a 50,000-row DOM guard. +- **Row filter**: case-insensitive substring across all values. +- **JSONL and Markdown exports.** Neither is formula-sanitized: JSON has types + and nothing evaluates it, and a leading `=` is inert in Markdown — Markdown + gets pipe/newline escaping instead. Both write the same flattened columns the + table shows (nested objects as dotted keys, nested arrays as JSON strings), + because the unflattened rows are never sent to the browser; see + "Known limitations". +- **Column visibility toggle** and **deep-linkable path selection** + (`#path=users.0.orders`). +- `/process` returns `preview_limit`, `total_cells` and `max_export_cells`, so + the badge reflects config and the Excel entry is greyed out before the click. +- CI (`.github/workflows/ci.yml`), `pyproject.toml` (ruff + pytest), + `requirements-dev.txt`, `requirements-redis.txt`, `.env.example`, `Makefile`, + and Node assertion suites for `static/js/app.js`. +- The rate-limit topology guard: `RATELIMIT_STORAGE_URI` is configurable at last, + and a production deployment must declare `WEB_CONCURRENCY` and `APP_REPLICAS`. + +### Changed + +- `alert()` About dialog replaced with an in-page modal that reads the version + from config; export dropdown is keyboard-accessible. +- Render auto-deploy now waits for CI (`autoDeployTrigger: checksPass`). +- License references corrected from MIT to GPL-3.0 (F17). + +### Removed + +- `find_candidate_arrays` and its four tests — dead code from the old candidates + handshake the JSON tree picker replaced (P10/D2). + +### Known limitations + +- **JSONL export is not a faithful copy of the input document.** Roadmap 4.3 + called it "lossless — original values". It writes values verbatim (no formula + prefixing, which was the security-relevant half of that decision), but over + the server's *flattened* projection: `{"tags": [1, 2]}` exports as + `"tags": "[1, 2]"`, and `{"meta": {"role": "x"}}` as `"meta.role": "x"`. + Only `csv_data` (flattened) reaches the browser for the full dataset — + `preview` is truncated and capped at `preview_limit` rows — so a + round-tripping export would mean shipping the original rows alongside the + flattened ones, doubling the payload and client memory that P2 and P12 exist + to reduce. Flagged for a maintainer decision rather than resolved either way. + +### Not included + +- **Opt-in HTTP Basic Auth gate** (roadmap 4.6). Decision D4 is still open and + needs maintainer sign-off. Nothing else in this release depends on it. + +## [1.1.0] - 2026-05 + +- JSON tree picker replaces the multi-array `candidates` handshake. +- JSONL support across file, paste and API input. +- Client-side column sorting, light/dark theme, CSV/TSV/Excel export. +- CSRF protection, SSRF validation with DNS resolution, rate limiting, and a + strict CSP with no inline scripts. diff --git a/CLAUDE.md b/CLAUDE.md index e38ed2a..450ade2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,28 +12,44 @@ A lightweight Flask web application that converts JSON/JSONL data into viewable ``` json-table-tool/ -├── app.py # Flask app factory, middleware registration -├── config.py # All settings via environment variables +├── app.py # Flask app factory, startup gates, gzip + error handlers +├── config.py # All settings via environment variables; is_production() ├── extensions.py # Flask-WTF (CSRF) and Flask-Limiter instances -├── security.py # SSRF protection (DNS validation) + security headers -├── helpers.py # Data processing (flatten, extract, JSONL parse, path selector) +├── security.py # SSRF (port allowlist + bounded DNS) + security headers +├── helpers.py # Data processing (flatten, extract, JSONL, sanitize, preview) ├── routes.py # Flask Blueprint with all route handlers ├── static/ │ ├── css/ │ │ └── style.css # All CSS (dark/light themes, components, utilities) │ └── js/ -│ └── app.js # All JavaScript (UI, sorting, export, theme, modal) +│ └── app.js # All JavaScript (UI, table, exports, theme, modals) ├── templates/ │ └── index.html # HTML structure only (refs external CSS/JS) ├── tests/ │ ├── conftest.py # Shared pytest fixtures (app, client) │ ├── test_helpers.py # Tests for data processing functions -│ ├── test_security.py # Tests for SSRF validation -│ └── test_routes.py # Integration tests for all routes +│ ├── test_security.py # SSRF validation, port allowlist, bounded DNS resolver +│ ├── test_routes.py # Integration tests for all routes + config gates +│ └── js/ # Node assertions against the real app.js (no build step) +│ ├── dom_stub.mjs +│ ├── test_export_sanitize.mjs +│ ├── test_render_caps.mjs +│ └── test_features.mjs ├── docs/ -│ └── code-health-final.md -├── requirements.txt # Python dependencies (pinned versions) +│ ├── code-health-final.md +│ ├── security-review-v1.2.md +│ ├── performance-review-v1.2.md +│ ├── roadmap-v1.2.md +│ └── export-budget-v1.2.md # How MAX_EXPORT_CELLS was measured +├── .github/workflows/ci.yml # lint, format, tests, JS assertions, pip-audit +├── pyproject.toml # ruff + pytest configuration +├── requirements.txt # Runtime dependencies (exact-pinned) +├── requirements-dev.txt # Test/lint tooling +├── requirements-redis.txt # Optional Redis client for shared rate-limit storage +├── .env.example # Every environment variable with its default +├── Makefile # test / test-js / lint / format / audit / coverage / run ├── render.yaml # Render.com deployment blueprint +├── CHANGELOG.md ├── AGENTS.md # Agent contract / quick-reference ├── MEMORY.md # Memory index for AI assistants ├── README.md @@ -42,15 +58,15 @@ json-table-tool/ ## Tech Stack -- **Backend:** Python 3.11+ (Render deploys with 3.14.5), Flask 3.0.0 (app factory pattern) +- **Backend:** Python 3.11+ (Render deploys with 3.14.5), Flask 3.1.3 (app factory pattern) - **Frontend:** Vanilla HTML/CSS/JavaScript (no frameworks, no build step) - **Security:** Flask-WTF 1.2.1 (CSRF), Flask-Limiter 3.5.0 (rate limiting) -- **HTTP client:** requests 2.31.0 -- **Excel export:** openpyxl 3.1.2 -- **Production server:** gunicorn 21.2.0 -- **Testing:** pytest 7.4.4 -- **Deployment:** Render.com (free tier, auto-deploy on push) -- **App version:** 1.1.0 (`config.APP_VERSION`, exposed via `/health`) +- **HTTP client:** requests 2.33.0 +- **Excel export:** openpyxl 3.1.5 +- **Production server:** gunicorn 22.0.0 +- **Testing:** pytest 9.0.3 (dev deps in `requirements-dev.txt`); Node assertions for `app.js` +- **Deployment:** Render.com (free tier; auto-deploy gated on CI via `autoDeployTrigger: checksPass`) +- **App version:** 1.2.0 (`Config.APP_VERSION`, exposed via `/health`) ## Development Setup @@ -84,20 +100,27 @@ python app.py **`helpers.py`** — Data processing: - `flatten_for_csv(data, parent_key, sep, _depth, max_depth)` — Recursively flattens nested dicts (dot notation). Lists are serialized via `json.dumps`. Stops recursing at `max_depth`. -- `extract_table_data(json_data)` — Extracts tabular rows from various JSON shapes (top-level array, dict containing an array of objects, nested dicts, or a single object). +- `flatten_rows(rows, max_depth)` — Flattens every row and accumulates column names in one pass; returns `(rows, sorted_columns)`. Replaces flatten-then-`get_all_columns`. +- `extract_table_data(json_data, _depth, max_depth)` — Extracts tabular rows from various JSON shapes (top-level array, dict containing an array of objects, nested dicts, or a single object). Depth-capped like `flatten_for_csv`. +- `sanitize_cell(value)` / `serialize_cell_value(value)` / `is_formula_trigger(value)` — Formula-injection defenses for **spreadsheet formats only** (CSV/TSV/XLSX). JSONL and Markdown exports must not use them. +- `preview_truncate(row)` — Builds a capped **copy** of a preview row (long strings, wide nested objects/arrays). Never mutates the source, so exports stay full-fidelity. +- `format_size(num_bytes)` — Renders a byte count in the largest non-zero unit. Lives here, not in `app.py`, so `routes.py` can use it without importing `app.py` (that cycle broke every routes-first import). - `get_all_columns(data)` — Returns sorted unique column names across rows. - `parse_jsonl(text)` — Parses JSON Lines (one JSON value per non-empty line), raising `ValueError` with line numbers on errors. -- `find_candidate_arrays(json_data)` — Discovers arrays of objects with their `path`, `length`, and first 5 `sample_keys` (used for the multi-array selector modal). - `extract_by_path(json_data, path)` — Navigates JSON by dot-notation path (`(root)` returns the document itself). **`routes.py`** — Flask Blueprint (`bp`): - `GET /` — Serves `index.html`. -- `GET /health` — Returns `{"status": "ok", "version": APP_VERSION}` for monitoring. -- `POST /process` — Parses JSON/JSONL from file/paste/API, returns preview + full CSV-ready data. Returns `{"needs_selection": true, "candidates": [...]}` if multiple arrays found and no `json_path` was provided. Rate-limited via `RATE_LIMIT_PROCESS`. -- `POST /export-csv` — Server-side CSV generation (fallback). Rate-limited via `RATE_LIMIT_EXPORT`. -- `POST /export-xlsx` — Server-side Excel generation via openpyxl. Rate-limited via `RATE_LIMIT_EXPORT`. +- `GET /health` — Returns `{"status": "ok", "version": APP_VERSION}` for monitoring (`version` omitted when `HEALTH_REVEAL_VERSION=0`). +- `GET /health/live` — Liveness. Checks nothing on purpose, so a failing dependency cannot cause a restart loop. +- `GET /health/ready` — Readiness. 200, or 503 with a `checks` map when the limiter storage or the Excel writer is unusable. +- `POST /process` — Parses JSON/JSONL from file/paste/API, returns preview + full CSV-ready data. Returns `{"needs_selection": true, "raw_json": ...}` when no `json_path` was provided, so the client can render the JSON tree picker. Rate-limited via `RATE_LIMIT_PROCESS`. Body is assembled by `_load_input()` and `_select_table_data()`. +- `POST /export-csv` — Generator-streamed CSV, deliberately **uncapped**. Rate-limited via `RATE_LIMIT_EXPORT`. +- `POST /export-xlsx` — Server-side Excel via openpyxl, capped by `MAX_EXPORT_CELLS` (400 when exceeded — never truncated). Writes no OS temp files. Rate-limited via `RATE_LIMIT_EXPORT`. -API-fetch specifics: `requests.get` is called with `stream=True`, `allow_redirects=False`, and a streaming size cap. Errors are logged but the user-facing message is a generic `"API request failed"` to avoid leaking internal hostnames. +The `/process` success payload is `{success, columns, preview, preview_limit, total_rows, total_cells, max_export_cells, csv_data, csv_columns}`. + +API-fetch specifics: `requests.get` is called with `stream=True`, `allow_redirects=False`, and a streaming size cap. The outbound header **name** is checked against an allowlist. Failures log a fixed string with no interpolation — the exception text contains the full URL, and a token can ride in the query string, fragment, userinfo *or* path. ### Frontend @@ -106,11 +129,12 @@ API-fetch specifics: `requests.get` is called with `stream=True`, `allow_redirec **`static/js/app.js`** — Vanilla JavaScript: - CSRF token management (meta tag → FormData / X-CSRFToken header) - Tab switching, file drag-drop, auth method selection, format toggle (JSON/JSONL) -- Client-side column sorting (click headers, asc/desc toggle) -- Client-side CSV/TSV export (no server round-trip needed) -- Server-side Excel export via `/export-xlsx` +- Client-side column sorting (click headers, asc/desc toggle), row filtering, "load more" pagination and column visibility toggles +- Client-side CSV, TSV, JSONL and Markdown export (no server round-trip needed) +- Server-side Excel export via `/export-xlsx`, greyed out ahead of time when `total_cells > max_export_cells` - Theme detection (`prefers-color-scheme`) with localStorage override -- Path selector modal when multiple candidate arrays are detected +- Lazily-built JSON tree picker modal for choosing which node becomes the table, with `#path=` deep links +- In-page About modal (no `alert()`), and a keyboard-accessible export dropdown **`templates/index.html`** — HTML structure only. References external CSS/JS via `url_for('static', ...)`. Includes CSRF meta tag, theme toggle button, format selector, export dropdown, and path selector modal. No inline scripts or styles (CSP enforced). @@ -118,10 +142,11 @@ API-fetch specifics: `requests.get` is called with `stream=True`, `allow_redirec 1. User provides JSON/JSONL (file / paste / API URL with optional auth). 2. Server validates input (SSRF check for API URLs, UTF-8 decoding, JSON/JSONL parsing, size caps). -3. If multiple candidate arrays found and no `json_path` is supplied, server returns the candidates so the UI can prompt the user to pick one. -4. Server returns `preview` (first `PREVIEW_ROW_LIMIT` rows) plus full `csv_data` / `csv_columns`. -5. Frontend renders the sortable preview table; nested objects render as mini tables. -6. Export: CSV/TSV generated client-side instantly; Excel via the server endpoint. +3. If no `json_path` is supplied, the server returns `raw_json` so the UI can render a tree picker and let the user choose a node. +4. Server returns `preview` (first `PREVIEW_ROW_LIMIT` rows, as a truncated **copy**) plus full-fidelity `csv_data` / `csv_columns` and the export budget. +5. Frontend renders the sortable preview table; nested objects render as mini tables, with render caps. +6. Response bodies over `GZIP_MIN_SIZE` are gzipped. +7. Export: CSV/TSV/JSONL/Markdown generated client-side instantly; Excel via the server endpoint. ## Configuration @@ -129,18 +154,31 @@ All settings live in `config.py`, configurable via environment variables: | Setting | Env Var | Default | Description | |---------|---------|---------|-------------| -| Secret key | `SECRET_KEY` | `dev-secret-key-change-in-production` | Flask/CSRF secret (change in production) | +| Secret key | `SECRET_KEY` | `dev-secret-key-change-in-production` | Flask/CSRF secret. The app **refuses to start** on the default when `APP_ENV=production` | +| Production signal | `APP_ENV` | unset | `production` is the single canonical signal (fail-fast, `Secure` cookie, topology guard). No alias is accepted | | Max upload size | `MAX_UPLOAD_SIZE` | 10 MB | Request body limit | | Preview rows | `PREVIEW_ROW_LIMIT` | 25 | Rows shown in preview table | -| API timeout | `API_FETCH_TIMEOUT` | 30s | Timeout for external API requests | +| API timeout | `API_FETCH_TIMEOUT` | 30s | Timeout for external API requests. Must stay below gunicorn's `--timeout` | | API max response | `API_FETCH_MAX_RESPONSE` | 10 MB | Max size for streamed API responses | -| Flatten depth | `FLATTEN_MAX_DEPTH` | 10 | Max recursion depth for CSV flattening | +| API ports | `API_ALLOWED_PORTS` | `80,443,8443` | Ports API fetch may connect to. Empty disables the check | +| DNS wait | `API_DNS_TIMEOUT` | 3s | Bounds how long a **request** waits, not the lookup | +| DNS workers | `API_DNS_MAX_WORKERS` | 4 | Concurrent lookups; this is the worker-starvation fix | +| DNS admission | `API_DNS_ADMISSION_TIMEOUT` | 1s | Wait for a permit before rejecting fast | +| Flatten depth | `FLATTEN_MAX_DEPTH` | 10 | Max recursion depth for flattening and extraction | +| Excel budget | `MAX_EXPORT_CELLS` | 250000 | XLSX-only cap in cells (`rows × columns`). `0` disables. CSV/TSV stay uncapped | +| Static cache | `STATIC_MAX_AGE` | 86400 | `Cache-Control` max-age for static assets (URLs carry `?v=APP_VERSION`) | +| gzip threshold | `GZIP_MIN_SIZE` | 1024 | Smallest body worth compressing | +| Health version | `HEALTH_REVEAL_VERSION` | on | `0` omits `version` from the health endpoints | +| Trust proxy | `TRUST_PROXY` | off | `1` installs `ProxyFix` for exactly one hop | +| Rate limit storage | `RATELIMIT_STORAGE_URI` | `memory://` | Counters are **process-local**; shared storage is required above 1 worker × 1 instance | +| Workers | `WEB_CONCURRENCY` | 1 | Single source of truth; start commands pass `--workers "$WEB_CONCURRENCY"`. Required under `APP_ENV=production` | +| Replicas | `APP_REPLICAS` | 1 | Mirrors `render.yaml`'s `numInstances`. Required under `APP_ENV=production` | | Rate limit (default) | `RATE_LIMIT_DEFAULT` | 120/minute | Global default rate limit | | Rate limit (process) | `RATE_LIMIT_PROCESS` | 30/minute | Rate limit on `/process` | | Rate limit (export) | `RATE_LIMIT_EXPORT` | 60/minute | Rate limit on export endpoints | | Debug mode | `FLASK_DEBUG` | off | Enable Flask debug mode | -Rate-limiter storage is in-memory (`RATELIMIT_STORAGE_URI = 'memory://'`); switch to Redis if running multiple workers and you want shared counters. +`.env.example` lists every variable with its default. Rate-limiter storage defaults to `memory://`, whose counters are **process-local** — the effective limit is multiplied by `workers × replicas`, so any deployment above one worker and one instance must set a shared `RATELIMIT_STORAGE_URI` (install `requirements-redis.txt`). Under `APP_ENV=production` the app refuses to start otherwise. ## Security @@ -154,42 +192,63 @@ Rate-limiter storage is in-memory (`RATELIMIT_STORAGE_URI = 'memory://'`); switc ## Testing -82 tests using pytest: +The passing command is the criterion, not a test count: ```bash -python -m pytest tests/ -v +python -m pytest tests/ -v # or: make test ``` Test files: -- `tests/test_helpers.py` (31 tests) — `flatten_for_csv`, `extract_table_data`, `get_all_columns`, `parse_jsonl`, `find_candidate_arrays`, `extract_by_path`. -- `tests/test_security.py` (16 tests) — URL validation with mocked DNS, private/loopback/link-local IP blocking, scheme checks. -- `tests/test_routes.py` (35 tests) — All route integration tests, security headers, JSONL, path selection, API-fetch SSRF/size/timeout/error-leak coverage, CSV/Excel export edge cases. +- `tests/test_helpers.py` — `flatten_for_csv`, `flatten_rows`, `extract_table_data` (incl. the depth guard), `get_all_columns`, `parse_jsonl`, `extract_by_path`, `preview_truncate`. +- `tests/test_security.py` — URL validation with mocked DNS, private/loopback/link-local IP blocking, scheme checks, the port allowlist, and the bounded DNS resolver (admission limit, permit accounting, fork lifecycle, and the *accepted* unbounded teardown). +- `tests/test_routes.py` — All route integration tests: security headers, JSONL, path selection, exports, formula injection, log hygiene via `caplog`, gzip, the config gates (`APP_ENV`, SECRET_KEY, integer validation), the rate-limit topology guard, and the health split. +- `tests/js/*.mjs` — Node assertions that load the real `static/js/app.js` in a stubbed DOM (`dom_stub.mjs`) and exercise the client export, render-cap and feature code. No build step and no npm dependencies; run with `make test-js`. -Fixtures in `tests/conftest.py` provide `app` (with `TESTING=True` and `WTF_CSRF_ENABLED=False`) and `client`. +Fixtures in `tests/conftest.py` provide `app` (with `TESTING=True` and `WTF_CSRF_ENABLED=False`) and `client`. `tests/test_routes.py` adds a `fresh_config` fixture that reloads `config` under a patched environment, because `Config` holds class attributes evaluated at import time. ## Linting / Formatting -No linting tools currently configured. Recommended: `ruff` for linting + formatting. +`ruff`, configured in `pyproject.toml` (target py311, line length 100, Python files only): + +```bash +ruff check . # or: make lint +ruff format --check . +ruff format . # or: make format +``` + +CI runs lint, format-check, pytest, the Node assertions, and `pip-audit -r requirements.txt`. ## Deployment ### Production Requirements (All Methods) -- Set `SECRET_KEY` to a random value (never use the dev default). +- Set `APP_ENV=production`. This is the single canonical production signal. +- Set `SECRET_KEY` to a random value. With `APP_ENV=production` the app **refuses + to start** on the dev default, an empty value, or an unset one. - Set `FLASK_DEBUG=0`. -- Use HTTPS (TLS termination via Nginx, cloud provider, or reverse proxy). +- Declare `WEB_CONCURRENCY` and `APP_REPLICAS`, and derive the start command's + `--workers` from `$WEB_CONCURRENCY`. Above one worker or one instance, set a + shared `RATELIMIT_STORAGE_URI` (and install `requirements-redis.txt`) — the app + refuses to start otherwise, because `memory://` counters are process-local. +- Set gunicorn's `--timeout` above `API_FETCH_TIMEOUT` (60 vs 30 by default). +- Use HTTPS (TLS termination via Nginx, cloud provider, or reverse proxy). Set + `TRUST_PROXY=1` behind a proxy you control so rate limiting and `Secure` + cookies see the real client and scheme. ### Render.com (PaaS) Configured via `render.yaml` blueprint: - Runtime: Python 3.14.5 - Build: `pip install -r requirements.txt` -- Start: `gunicorn "app:create_app()" --bind 0.0.0.0:$PORT` -- `SECRET_KEY` is generated by Render; auto-deploy on push; free tier; no persistent storage. +- Start: `gunicorn "app:create_app()" --bind 0.0.0.0:$PORT --workers "$WEB_CONCURRENCY" --timeout 60` +- `SECRET_KEY` is generated by Render; `APP_ENV=production`, `WEB_CONCURRENCY=1` + and `APP_REPLICAS=1` are declared in the blueprint; `numInstances: 1`. +- `autoDeployTrigger: checksPass` — a push with failing or missing CI checks does + not deploy. Free tier; no persistent storage. ### Own Server (Gunicorn + Systemd + Nginx) -1. **Gunicorn** runs the app: `gunicorn "app:create_app()" --bind 127.0.0.1:8000 --workers 4` +1. **Gunicorn** runs the app: `gunicorn "app:create_app()" --bind 127.0.0.1:8000 --workers "$WEB_CONCURRENCY" --timeout 60` 2. **Systemd** manages the process (auto-restart, boot start) — see `README.md` for the unit file. 3. **Nginx** reverse-proxies and handles TLS termination; can also serve `/static/` directly. @@ -211,7 +270,7 @@ Railway.app and Fly.io are also supported — see `README.md` for CLI commands. ## Common Tasks ### Adding a new route -Add the handler to `routes.py` on the `bp` Blueprint. Apply `@limiter.limit()` if needed (use a lambda reading from `current_app.config` to keep the limit configurable). Follow existing patterns: `jsonify()` for responses, try/except around external calls, generic error messages with proper HTTP status codes. +Add the handler to `routes.py` on the `bp` Blueprint. Apply `@limiter.limit()` if needed (use a lambda reading from `current_app.config` to keep the limit configurable). Follow existing patterns: `jsonify()` for responses, try/except around external calls, generic error messages with proper HTTP status codes. Re-raise `HTTPException` before the generic `except Exception` so Flask's JSON error handlers still run. If the route returns payload data, add its endpoint to `security.NO_STORE_ENDPOINTS`. ### Modifying the UI - **CSS:** Edit `static/css/style.css`. Use existing CSS custom properties. Add light-theme overrides under `:root.light` if needed. @@ -225,12 +284,40 @@ Add the handler to `routes.py` on the `bp` Blueprint. Apply `@limiter.limit()` i 4. Add the JS visibility toggle in `app.js` (auth-method switching section). ### Adding a new export format -1. Client-side: add a handler in `app.js` (extend `downloadDelimited()` or add a new function). -2. Server-side: add a route in `routes.py` and a button in the export dropdown. -3. Add any new dependency to `requirements.txt` with a pinned version. +1. **Decide the sanitization policy first.** Spreadsheet-compatible formats + (anything a spreadsheet will open and evaluate) must route every cell through + `helpers.sanitize_cell`, or pin the cell type if the format has one. Lossless + or plain-text formats (JSONL, Markdown) must **not** — a quote prefix would + corrupt the data while protecting nothing. See MEMORY.md, 2026-08-21. +2. Client-side: add a `build*Chunks()` builder in `app.js` and dispatch it from + the export-dropdown handler. Return chunks, not one giant string. +3. Server-side (if needed): add a route in `routes.py`. Stream it with a + generator unless the format genuinely cannot be streamed; if it cannot, give + it a measured budget the way `MAX_EXPORT_CELLS` works, and never truncate — + refuse with a 400 and advertise the limit from `/process`. +4. Add the button to the export dropdown in `index.html` with + `role="menuitem"`. +5. Add assertions to `tests/js/test_features.mjs` covering the sanitization + decision explicitly, in both directions. +6. Add any new dependency to `requirements.txt` with an exact pin. ### Changing configuration defaults -Edit `config.py`. All values read from `os.environ.get()` with defaults. +Edit `config.py`. Read integers through `env_int()` (and integer lists through +`env_int_set()`) so a typo names the variable instead of raising a bare +`ValueError` at import. Add the variable to `.env.example`, the table above, and +the README table. Gate any production-only behavior on `is_production()` — never +on `not DEBUG`, and never on a second env-var spelling. ### Adding a dependency -Add to `requirements.txt` with a pinned version (e.g., `package==1.2.3`). +Add to `requirements.txt` with an exact pin (e.g. `package==1.2.3`). Test and +lint tooling goes in `requirements-dev.txt`; anything only a specific deployment +shape needs goes in its own file (see `requirements-redis.txt`). Re-run +`pip-audit -r requirements.txt` and the full suite in the same commit — the +convention is a dedicated bump commit, not a bump ridden along with a feature. + +### Re-deriving the Excel export budget +`MAX_EXPORT_CELLS` is a measured number, not a chosen one. Follow the method in +`docs/export-budget-v1.2.md` (fresh process per measured request, `ru_maxrss` +converted for the platform, delta across two runs), re-fit against the narrowest +aspect ratio you care about, and re-run a confirming point at the value you +intend to ship. Record the new data in that file. diff --git a/MEMORY.md b/MEMORY.md index ed82583..8e1154c 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -31,7 +31,7 @@ Keep entries short — if it grows past ~10 lines, it probably belongs in `READM **What:** The app must never persist user-submitted JSON to disk, database, or any external system. The only writes are stdout logs (and those deliberately omit payloads). **Why:** Designed as an internal tool for handling potentially sensitive payloads (API responses, exports). The Render free tier deliberately has no persistent disk to enforce this physically. -**How to apply:** Reject any change that adds a DB driver, file write of payload bytes, third-party analytics, or request-body logging. `logger.warning("API request failed: %s", e)` is fine; `logger.warning("payload was: %s", body)` is not. +**How to apply:** Reject any change that adds a DB driver, file write of payload bytes, third-party analytics, or request-body logging. Log **fixed strings**: `logger.warning('API request failed')` is fine; interpolating the payload, the URL, or the exception is not — `requests`' exception text embeds the full URL, which can carry a token (see the 2026-08-21 log-hygiene entry). ### 2026-05-12 — Gunicorn must call the factory, not a module-level `app` (area: deploy) @@ -63,10 +63,10 @@ Keep entries short — if it grows past ~10 lines, it probably belongs in `READM **Why:** All state-changing routes accept browser form posts, so CSRF is mandatory. Disabling it in tests keeps fixtures simple — production behavior is exercised manually and via the security headers test. **How to apply:** When adding a route that mutates state or returns sensitive data, it inherits CSRF protection automatically. Don't add `@csrf.exempt` without justification. When testing CSRF behavior, do so in a dedicated test that flips `WTF_CSRF_ENABLED` back on. -### 2026-05-12 — Multi-array JSON triggers a path-selector handshake (area: backend / ux) +### 2026-05-12 — Unselected JSON triggers the tree-picker handshake (area: backend / ux) -**What:** `/process` returns `{"needs_selection": true, "candidates": [...]}` (HTTP 200) when `find_candidate_arrays` reports more than one array of objects in the payload. The frontend opens a modal; the user picks; the request is re-submitted with `json_path` set to the chosen dotted path. -**Why:** The original heuristic (`extract_table_data`) silently picked the first array it found, which surfaced the wrong data for nested API responses. Returning candidates is more honest than guessing. +**What:** `/process` returns `{"needs_selection": true, "raw_json": }` (HTTP 200) whenever no `json_path` was supplied. The frontend renders a JSON **tree picker** over `raw_json`; the user clicks any array or object node; the request is re-submitted with `json_path` set to the chosen dotted path. +**Why:** The original heuristic (`extract_table_data`) silently picked the first array it found, which surfaced the wrong data for nested API responses. Handing the client the document and letting the user point at a node is more honest than guessing — and unlike the earlier `candidates` list it can reach any level, not just arrays of objects. **How to apply:** Don't "fix" the heuristic by being smarter — the selection prompt *is* the fix. The sentinel `'(root)'` is used when the top-level value is itself a list. ### 2026-05-12 — `flatten_for_csv` has a recursion-depth cap (area: backend) @@ -101,10 +101,103 @@ Keep entries short — if it grows past ~10 lines, it probably belongs in `READM ### 2026-05-12 — Pinned dependencies are deliberate (area: deploy) -**What:** `requirements.txt` uses exact `==` pins (Flask 3.0.0, requests 2.31.0, gunicorn 21.2.0, Flask-WTF 1.2.1, Flask-Limiter 3.5.0, pytest 7.4.4, openpyxl 3.1.2). +**What:** `requirements.txt` uses exact `==` pins (Flask 3.1.3, requests 2.33.0, gunicorn 22.0.0, Flask-WTF 1.2.1, Flask-Limiter 3.5.0, openpyxl 3.1.5). Test tooling lives in `requirements-dev.txt`, and the optional Redis client in `requirements-redis.txt`. **Why:** Render auto-deploys on push. Loose pins + auto-deploy = surprise breakage. Exact pins keep deploys reproducible and make security audits possible. **How to apply:** Bump versions intentionally in a dedicated commit, run the full test suite, and verify the Render build before merging. Don't bump on a feature commit "while we're in here". +### 2026-08-21 — Spreadsheet exports are formula-sanitized; JSONL and Markdown are not (area: security) + +**What:** Values starting with `=`, `+`, `-`, `@`, tab, CR or LF are formula triggers (CWE-1236). CSV/TSV prefix them with a single quote; XLSX instead pins the cell's `data_type` to `'s'`, because openpyxl serializes a leading `=` as a *formula cell* and Excel then runs it without the CSV warning. JSONL and Markdown exports are deliberately exempt. +**Why:** The tool's whole job is turning untrusted API/file JSON into spreadsheets, so an attacker who controls a cell controls the exported file's formulas. The exemptions are not oversights: JSON carries types and nothing evaluates it, so a quote prefix would corrupt data while protecting nothing; Markdown does not evaluate `=` either, but an unescaped pipe or newline breaks the table, so it gets Markdown-specific escaping. +**How to apply:** Any new **spreadsheet-compatible** export must route cells through `helpers.sanitize_cell` (or pin the data type, for typed formats). Any new **non-spreadsheet** format (JSONL, Markdown, ...) must not. There are four sanitized paths today — two server routes plus the client CSV and TSV builders — and `tests/js/test_export_sanitize.mjs` exists so none of them can regress silently. + +### 2026-08-21 — The API-fetch failure log is a fixed string (area: security) + +**What:** `logger.warning('API request failed')` — no interpolation, ever. +**Why:** `requests`' exception text embeds the full URL. With query-param auth the token rides in that URL, so the old `'API request failed: %s'` wrote secrets to stdout. Query strings, fragments, userinfo **and paths** can all carry tokens, so a redaction helper that preserves the path is not sufficient. +**How to apply:** Never add the URL, the exception, or any request field to a log line on this path. `caplog` tests assert no URL component reaches the logs; keep them passing. + +**Owner / source:** security review F3/F9. + +### 2026-08-21 — DNS is bounded in *concurrency*, not in execution (area: security / performance) + +**What:** Lookups run on a shared, fixed-size `ThreadPoolExecutor` created lazily inside the worker (it records its pid, so a pool inherited across a fork is replaced). The admission permit is taken *before* `submit` and released from the future's **done-callback**, never from the caller's `finally`. +**Why:** `getaddrinfo` takes no timeout and cannot be cancelled. `API_DNS_TIMEOUT` bounds only how long the *request* waits; the lookup keeps running. Releasing the permit on caller timeout would re-admit work into an already-blocked pool, which is exactly how it saturates under repeated slow-DNS requests. **Teardown is not bounded by anything this code controls** — glibc's defaults are ~5s per nameserver × 2 attempts × every nameserver in `resolv.conf`, so tens of seconds is the realistic worst case. +**How to apply:** Do **not** describe teardown as bounded in any doc, comment or test. Assert the caller wait and the admission error instead. `options timeout:2 attempts:1` in the container's `resolv.conf` is a best-effort narrowing; a killable subprocess resolver is the only real bound and is not in v1.2. + +**Owner / source:** security review F6.1, performance review P7. + +### 2026-08-21 — `APP_ENV=production` is the only production signal (area: deploy / security) + +**What:** One env var, read through one helper (`config.is_production()`), gating the SECRET_KEY fail-fast, `SESSION_COOKIE_SECURE`, and the rate-limit topology guard. +**Why:** Two accepted spellings let a deployment satisfy one gate and silently miss another — e.g. passing the secret-key check with `Secure` cookies still off, a live vulnerability produced purely by the inconsistency. And production is never inferred from `not DEBUG`: the documented local run `python app.py` has `DEBUG=False`, so that would block ordinary development. +**How to apply:** New production-only behavior calls `is_production()`. Never add `PRODUCTION=true`, `ENV=prod`, or any alias — a test asserts `PRODUCTION=true` alone is *not* honored. + +**Owner / source:** security review F7/F16. + +### 2026-08-21 — `memory://` rate-limit counters multiply by workers × replicas (area: deploy) + +**What:** The default storage is process-local, so N workers on M instances enforce N×M times the configured limit. `RATELIMIT_STORAGE_URI` is configurable (it was hardcoded before v1.2). Under `APP_ENV=production` the app refuses to start unless `WEB_CONCURRENCY` and `APP_REPLICAS` are declared, the start command's `--workers` agrees with `WEB_CONCURRENCY`, and storage is shared whenever either count exceeds one. +**Why:** Defaults of 1 fail *open* — an undeclared 4-worker deployment reads as single-worker, which is exactly where the guard matters most. `WEB_CONCURRENCY` is the single source of truth because gunicorn reads it natively, so the number the app validates cannot drift from the number gunicorn runs. +**How to apply:** To run more than one worker or instance: install `requirements-redis.txt`, set `RATELIMIT_STORAGE_URI=redis://…`, then raise the counts. Never write a bare `--workers N` into a start command — derive it from `$WEB_CONCURRENCY`. `APP_REPLICAS` must mirror `render.yaml`'s `numInstances`. + +**Owner / source:** security review F12, roadmap 2.10. + +### 2026-08-21 — API fetch is restricted to ports 80, 443 and 8443 (area: security) + +**What:** `API_ALLOWED_PORTS` (default `80,443,8443`), checked before DNS so a rejected URL costs no lookup. An empty value disables the check. +**Why:** Only the resolved IP was validated, so `http://public.example.com:22` or `:6379` passed and the tool would connect to any port on any public host. +**How to apply:** Widen the list via env rather than in code, and keep the check ahead of resolution. + +**Owner / source:** security review F6.2, decision D5. + +### 2026-08-21 — The XLSX export budget is measured, in cells, and on by default (area: performance) + +**What:** `MAX_EXPORT_CELLS` (default 250,000) caps Excel exports only. CSV/TSV are generator-streamed and stay uncapped. +**Why:** openpyxl memory tracks `rows × columns`, not rows — at equal cell counts a narrow, tall sheet costs *more* (84.3 MiB at 50k×3 vs 73.6 at 15k×10), so a row limit says almost nothing about the footprint. The default comes from the measurement in `docs/export-budget-v1.2.md`, not from feel. An unlimited default would leave a High finding unmitigated; uncapped CSV/TSV is what keeps the export contract as wide as the input contract. +**How to apply:** Re-derive the number whenever the measurement is re-run — do not round it to something tidy. Never truncate an oversized export: `/process` advertises `total_cells`/`max_export_cells` so the client greys Excel out beforehand, and `/export-xlsx` returns 400 for direct callers. And keep exports diskless: openpyxl's `write_only` mode writes worksheet parts to OS temp files, and `SpooledTemporaryFile` is either pointless (its default `max_size=0` never rolls over) or disk-backed. + +**Owner / source:** performance review P3, decision D6. + +### 2026-08-21 — gunicorn's `--timeout` must exceed `API_FETCH_TIMEOUT` (area: deploy) + +**What:** Every documented invocation sets `--timeout 60` against a default `API_FETCH_TIMEOUT` of 30s. +**Why:** gunicorn's default timeout is also 30s, so a slow API fetch raced the worker kill: the worker was SIGKILLed mid-response and the client saw a 502 instead of the timeout message. +**How to apply:** If you raise `API_FETCH_TIMEOUT`, raise `--timeout` with it — roughly double is the documented margin. + +**Owner / source:** performance review P9. + +### 2026-08-21 — The preview is a truncated copy; exports are not (area: backend) + +**What:** `helpers.preview_truncate` builds a *new* row capping long strings, nested objects and nested arrays. `table_data` and `csv_data` are never mutated. +**Why:** Preview rows used to carry full-fidelity nested structures, so a 50k-key object or a 5 MB string cell froze the tab. Truncating in place would have silently corrupted every export. +**How to apply:** Anything that trims data for display must build a projection. Tests assert the server CSV and XLSX exports still contain the untruncated values — keep them. + +**Owner / source:** performance review P2.2/P5. + + +--- + +### 2026-08-22 — `routes.py` must never import `app.py` (area: backend) + +**What:** Shared utilities go in `helpers.py` (or another leaf module), never in `app.py`. `helpers.format_size` is there for exactly this reason. +**Why:** `create_app()` imports `bp` from `routes.py`, so a module-level `from app import ...` in `routes.py` makes `import routes` re-enter a half-initialized module and raise `ImportError`. It hid for a while because gunicorn's `app:create_app()` imports app-first, which happens to work — only routes-first entry points broke. +**How to apply:** `TestNoImportCycle` in `tests/test_routes.py` imports each module first in a fresh subprocess and AST-checks that `routes.py` does not import `app`. If you need something from `app.py` in a route, move it down, don't import up. + +**Owner / source:** CodeRabbit review on the v1.2.0 PR. + + +--- + +### 2026-08-22 — A blank element in an integer-list env var is an error, not a default (area: deploy / security) + +**What:** `config.env_int_set` rejects `80,,443`, `80,443,` and a lone `,`. Only an *unset* variable selects the default; only a fully empty value disables the check. +**Why:** `security.validate_url` reads an empty allowlist as "no port restriction". Skipping blank elements meant `API_ALLOWED_PORTS=,` silently removed the outbound port restriction, and `80,,443` silently narrowed it — both from a typo, with no startup error. +**How to apply:** Any list-valued setting whose empty state weakens a check must fail loudly on a malformed element. Never `if part.strip()` your way past bad input in a security setting. + +**Owner / source:** CodeRabbit review on the v1.2.0 PR. + + --- ## Conventions for Adding Entries diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..da5ebc8 --- /dev/null +++ b/Makefile @@ -0,0 +1,57 @@ +# Developer entry points. Everything here is also what CI runs. + +VENV ?= venv +PY ?= $(VENV)/bin/python +PIP ?= $(VENV)/bin/pip + +.PHONY: help venv install test test-js lint format audit coverage run check clean + +help: + @echo "make install - create the venv and install dev dependencies" + @echo "make test - run the Python test suite" + @echo "make test-js - run the Node assertions for static/js/app.js" + @echo "make lint - ruff check + ruff format --check" + @echo "make format - ruff format (rewrites files)" + @echo "make audit - pip-audit against the runtime requirements" + @echo "make coverage - test suite with a coverage report" + @echo "make run - start the development server on :5000" + @echo "make check - lint + test + test-js + audit (what CI runs)" + +venv: + test -d $(VENV) || python3 -m venv $(VENV) + +install: venv + $(PIP) install --upgrade pip + $(PIP) install -r requirements-dev.txt + +test: + $(PY) -m pytest tests/ -v + +test-js: + node tests/js/test_export_sanitize.mjs + node tests/js/test_render_caps.mjs + node tests/js/test_features.mjs + +lint: + $(VENV)/bin/ruff check . + $(VENV)/bin/ruff format --check . + +format: + $(VENV)/bin/ruff format . + $(VENV)/bin/ruff check . --fix + +audit: + $(VENV)/bin/pip-audit -r requirements.txt + +coverage: + $(VENV)/bin/coverage run -m pytest tests/ + $(VENV)/bin/coverage report -m + +run: + $(PY) app.py + +check: lint test test-js audit + +clean: + find . -type d -name __pycache__ -prune -exec rm -rf {} + + rm -rf .pytest_cache .coverage .ruff_cache diff --git a/README.md b/README.md index 3a3b1ac..eccfb76 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ A lightweight web tool to convert JSON data into viewable tables with CSV export ![Python](https://img.shields.io/badge/Python-3.11+-blue) ![Flask](https://img.shields.io/badge/Flask-3.0-green) -![License](https://img.shields.io/badge/License-MIT-yellow) +![License](https://img.shields.io/badge/License-GPL--3.0-yellow) ## Features @@ -20,10 +20,11 @@ A lightweight web tool to convert JSON data into viewable tables with CSV export - Query Parameter Token - **Data Processing** - - Handles nested JSON objects - - Displays nested data as expandable tables - - Preview first 25 rows - - Export ALL rows to CSV + - Handles nested JSON objects and JSON Lines + - Displays nested data as expandable tables (with render caps, so a huge cell cannot freeze the tab) + - Preview the first `PREVIEW_ROW_LIMIT` rows, then "Load next 500" / "Load all" + - Filter rows, sort columns, hide columns + - Export **all** rows to CSV, TSV, JSONL, Markdown or Excel - **Privacy First** - No data storage - everything processed in-memory @@ -111,7 +112,11 @@ git push -u origin main - **Branch**: `main` - **Runtime**: `Python 3` - **Build Command**: `pip install -r requirements.txt` - - **Start Command**: `gunicorn "app:create_app()" --bind 0.0.0.0:$PORT` + - **Start Command**: `gunicorn "app:create_app()" --bind 0.0.0.0:$PORT --workers "$WEB_CONCURRENCY" --timeout 60` + - **Environment**: `SECRET_KEY` (click *Generate* — the app **refuses to start** + under `APP_ENV=production` with the development default), `APP_ENV=production`, + `WEB_CONCURRENCY=1`, `APP_REPLICAS=1` + (see [Deployment topology](#deployment-topology-and-rate-limiting)) 4. Select **Free** plan 5. Click **"Create Web Service"** @@ -181,12 +186,31 @@ python -m venv venv source venv/bin/activate pip install -r requirements.txt -# Set production environment variables -export SECRET_KEY="your-random-secret-key-here" +# Set production environment variables. +# +# SECRET_KEY must be a random value you generate, not a literal copied from this +# README. The startup gate rejects the dev default and an empty value, but it +# cannot tell a real secret from a memorable one someone pasted: +# +# python -c "import secrets; print(secrets.token_urlsafe(48))" +# +# Every SECRET_KEY placeholder below means "the output of that command", kept +# out of the shell history and out of version control. +export SECRET_KEY="$(python -c 'import secrets; print(secrets.token_urlsafe(48))')" export FLASK_DEBUG=0 - -# Run with gunicorn -gunicorn "app:create_app()" --bind 0.0.0.0:8000 --workers 4 +export APP_ENV=production + +# Deployment topology. memory:// rate-limit counters are process-local, so the +# effective limit is multiplied by workers x replicas. One worker and one +# instance is the default; see "Deployment topology and rate limiting" below +# before raising either. +export WEB_CONCURRENCY=1 +export APP_REPLICAS=1 + +# Run with gunicorn. --workers comes from WEB_CONCURRENCY so the running count +# and the declared count cannot drift, and --timeout stays above +# API_FETCH_TIMEOUT (default 30s). +gunicorn "app:create_app()" --bind 0.0.0.0:8000 --workers "$WEB_CONCURRENCY" --timeout 60 ``` #### Systemd Service (Auto-Start on Boot) @@ -202,9 +226,12 @@ After=network.target User=www-data Group=www-data WorkingDirectory=/opt/json-table-tool -Environment="SECRET_KEY=your-random-secret-key-here" +Environment="SECRET_KEY=" Environment="FLASK_DEBUG=0" -ExecStart=/opt/json-table-tool/venv/bin/gunicorn "app:create_app()" --bind 127.0.0.1:8000 --workers 4 +Environment="APP_ENV=production" +Environment="WEB_CONCURRENCY=1" +Environment="APP_REPLICAS=1" +ExecStart=/opt/json-table-tool/venv/bin/gunicorn "app:create_app()" --bind 127.0.0.1:8000 --workers ${WEB_CONCURRENCY} --timeout 60 Restart=always [Install] @@ -253,16 +280,48 @@ COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt COPY . . EXPOSE 8000 -CMD ["gunicorn", "app:create_app()", "--bind", "0.0.0.0:8000", "--workers", "4"] +ENV APP_ENV=production +ENV WEB_CONCURRENCY=1 +ENV APP_REPLICAS=1 +# --workers is derived from WEB_CONCURRENCY (shell form so it expands), and +# --timeout stays above API_FETCH_TIMEOUT. +CMD gunicorn "app:create_app()" --bind 0.0.0.0:8000 --workers "$WEB_CONCURRENCY" --timeout 60 ``` ```bash docker build -t json-table-tool . docker run -p 8000:8000 \ - -e SECRET_KEY="your-random-secret-key-here" \ + -e SECRET_KEY="$(python -c 'import secrets; print(secrets.token_urlsafe(48))')" \ json-table-tool ``` +### Deployment topology and rate limiting + +Flask-Limiter's default `memory://` storage keeps its counters **inside one +process**. The effective limit is therefore multiplied by `workers x replicas`, +not by workers alone: four workers on two instances enforce eight times the +configured limit. + +The supported default is **one worker and one instance**. To run more: + +1. Install the Redis client: `pip install -r requirements.txt -r requirements-redis.txt` +2. Set `RATELIMIT_STORAGE_URI=redis://...` +3. Raise `WEB_CONCURRENCY` (and `APP_REPLICAS`, mirroring `numInstances`) + +Under `APP_ENV=production` the app refuses to start if `WEB_CONCURRENCY` or +`APP_REPLICAS` is undeclared, if a `--workers N` in the start command disagrees +with `WEB_CONCURRENCY`, or if either count exceeds one while storage is still +`memory://`. Outside production the same conditions log a warning instead. + +**HTTPS is required on every deployment path.** API keys, bearer tokens and +basic-auth passwords are POSTed from the browser to this app; without TLS they +are exposed on the wire. + +**Timeout invariant:** gunicorn's `--timeout` must stay above +`API_FETCH_TIMEOUT` (a factor of two is the documented margin). gunicorn's +default of 30s equals the default `API_FETCH_TIMEOUT`, so a slow API fetch +raced the worker kill and surfaced as a 502. + #### Docker Compose ```yaml @@ -273,8 +332,12 @@ services: ports: - "8000:8000" environment: - - SECRET_KEY=your-random-secret-key-here + # See the SECRET_KEY note above: generate this, do not copy a literal. + - SECRET_KEY=${SECRET_KEY:?set SECRET_KEY in .env or the environment} - FLASK_DEBUG=0 + - APP_ENV=production + - WEB_CONCURRENCY=1 + - APP_REPLICAS=1 - RATE_LIMIT_PROCESS=30/minute - RATE_LIMIT_EXPORT=60/minute restart: unless-stopped @@ -284,6 +347,14 @@ services: docker compose up -d ``` +This compose file terminates **no TLS** — it publishes plain HTTP on 8000, which +is only appropriate behind something that does. Credentials for the API-fetch +feature are POSTed from the browser, so put this behind the Nginx/Let's Encrypt +front end from the section above (or your platform's load balancer) and do not +publish port 8000 to the internet directly. With TLS terminated upstream, also +set `TRUST_PROXY=1` so rate limiting and the `Secure` cookie flag see the real +client address and scheme. + ### Railway.app ```bash @@ -316,6 +387,25 @@ Set these in production for any deployment method: | `RATE_LIMIT_PROCESS` | No | `30/minute` | Rate limit on /process endpoint | | `RATE_LIMIT_EXPORT` | No | `60/minute` | Rate limit on export endpoints | | `RATE_LIMIT_DEFAULT` | No | `120/minute` | Default rate limit for all routes | +| `APP_ENV` | **Yes** (prod) | unset | Set to `production`. The single canonical production signal: enables the SECRET_KEY fail-fast, the `Secure` session cookie and the rate-limit topology guard. No alias (`PRODUCTION=true`, …) is accepted | +| `WEB_CONCURRENCY` | **Yes** (prod) | `1` | Worker count, and the single source of truth for it — start commands pass `--workers "$WEB_CONCURRENCY"` | +| `APP_REPLICAS` | **Yes** (prod) | `1` | Instance count; must mirror `render.yaml`'s `numInstances` | +| `RATELIMIT_STORAGE_URI` | No | `memory://` | Counters are process-local. Required to be shared (`redis://…`) above 1 worker × 1 instance | +| `TRUST_PROXY` | No | `0` | `1` trusts `X-Forwarded-*` from exactly one hop. Enable only behind a proxy you control | +| `API_ALLOWED_PORTS` | No | `80,443,8443` | Ports the API-fetch feature may connect to. Empty disables the check | +| `API_DNS_TIMEOUT` | No | `3` | Seconds a **request** waits for DNS. Does not bound the lookup itself | +| `API_DNS_MAX_WORKERS` | No | `4` | Concurrent DNS lookups | +| `API_DNS_ADMISSION_TIMEOUT` | No | `1` | Seconds to wait for a DNS permit before rejecting | +| `FLATTEN_MAX_DEPTH` | No | `10` | Max recursion depth for flattening and extraction | +| `MAX_EXPORT_CELLS` | No | `250000` | Excel-only budget in cells (`rows × columns`). `0` disables it. CSV/TSV are streamed and uncapped | +| `STATIC_MAX_AGE` | No | `86400` | `Cache-Control` max-age for static assets (URLs are version-busted) | +| `GZIP_MIN_SIZE` | No | `1024` | Smallest response body worth compressing | +| `HEALTH_REVEAL_VERSION` | No | `1` | Set to `0` to omit `version` from the health endpoints | + +`.env.example` lists every variable with its default and the reasoning behind it. + +**HTTPS is required on every deployment path** — API keys, bearer tokens and +basic-auth passwords are POSTed from the browser to this app. --- @@ -345,8 +435,14 @@ Set these in production for any deployment method: - After conversion, click **"Export"** to see format options: - **CSV** — Comma-separated values (generated instantly in your browser) - **TSV** — Tab-separated values (generated instantly in your browser) + - **JSONL** — one JSON object per line, over the same flattened columns the + table shows, with values unescaped (no formula prefixing applied) + - **Markdown** — a Markdown table; `|`, `\` and line breaks are escaped so a + value cannot break the table, but no formula prefixing is applied - **Excel** — `.xlsx` file via server-side generation - All formats export ALL rows (not just the preview) +- Excel is greyed out when the dataset exceeds `MAX_EXPORT_CELLS`; CSV and TSV + are streamed and have no such limit, so every dataset stays exportable --- @@ -392,9 +488,24 @@ Set these in production for any deployment method: - **No database**: No persistence layer configured - **CSRF protection**: All POST routes protected via Flask-WTF tokens - **SSRF prevention**: API fetch validates DNS, blocks private/internal IPs -- **Rate limiting**: Configurable per-route rate limits (Flask-Limiter) -- **Security headers**: CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy -- **HTTPS**: Render provides free SSL/TLS; use Nginx/Let's Encrypt for self-hosted +- **Rate limiting**: Configurable per-route rate limits (Flask-Limiter), per client IP + behind a trusted proxy (`TRUST_PROXY=1`) +- **Security headers**: CSP (`script-src 'self'`, `object-src 'none'`, `base-uri 'self'`, + `frame-ancestors 'none'`, `form-action 'self'`, `upgrade-insecure-requests`), + HSTS on secure requests, `Permissions-Policy`, COOP/CORP, X-Frame-Options, + X-Content-Type-Options, Referrer-Policy, and `Cache-Control: no-store` on data responses +- **Formula-injection defense**: values starting with `=`, `+`, `-`, `@`, tab, CR or LF + are neutralized in CSV, TSV and Excel exports, so an untrusted value cannot become a + live formula when the file is opened. JSONL and Markdown deliberately do *not* + get that prefix: JSON carries types and nothing evaluates it, and a leading `=` + is inert in Markdown, so prefixing there would corrupt values while protecting + nothing. (Markdown still escapes `|`, `\` and line breaks — that is table + structure, not formula defense. Anyone pasting a Markdown or JSONL export into + a spreadsheet is back to unprotected input: export CSV, TSV or Excel for that.) +- **Startup gates**: with `APP_ENV=production`, the app refuses to start on the + development `SECRET_KEY` or with a rate-limit topology it cannot enforce +- **HTTPS**: Render provides free SSL/TLS; use Nginx/Let's Encrypt for self-hosted. + **Required on every deployment path** — credentials are POSTed from the browser - **Stateless**: Each request is independent, no session state --- @@ -421,19 +532,41 @@ Set these in production for any deployment method: ## Development ```bash +# One-time setup (creates ./venv and installs dev dependencies) +make install + # Run in debug mode export FLASK_DEBUG=1 -python app.py - -# Run tests -python -m pytest tests/ -v +make run # or: python app.py + +# Everything CI runs +make check # lint + test + test-js + audit + +# Individually +make test # python -m pytest tests/ -v +make test-js # Node assertions for static/js/app.js (no npm install needed) +make lint # ruff check + ruff format --check +make format # ruff format + ruff check --fix +make audit # pip-audit -r requirements.txt +make coverage # test suite with a coverage report ``` +Copy `.env.example` to `.env` for a local configuration reference; every value +there is the built-in default. + +**Performance notes.** `/process` returns the full flattened dataset so exports +need no server round trip and nothing is persisted — responses over +`GZIP_MIN_SIZE` are gzipped to keep that affordable. The preview rows are a +truncated *copy*, so exports keep full fidelity. Excel exports are bounded by +`MAX_EXPORT_CELLS`, measured rather than guessed +(`docs/export-budget-v1.2.md`); CSV and TSV stream and stay uncapped. The JSON +tree picker builds children only when a node is opened. + --- ## License -MIT License - Feel free to modify and use internally. +GNU General Public License v3.0 — see [`LICENSE`](LICENSE) for the full text. --- diff --git a/app.py b/app.py index 2befda8..1cafe5e 100644 --- a/app.py +++ b/app.py @@ -1,25 +1,319 @@ """JSON Table Converter - Flask application factory.""" -from flask import Flask -from config import Config +import gzip +import logging +import os +import sys + +from flask import Flask, current_app, jsonify, request +from flask_wtf.csrf import CSRFError +from werkzeug.middleware.proxy_fix import ProxyFix + +from config import DEV_SECRET_KEY, Config, is_production from extensions import csrf, limiter +from helpers import format_size from security import apply_security_headers +logger = logging.getLogger(__name__) + + +def _assert_production_secret_key(app): + """ + Refuse to start a production deployment on the publicly known dev key (F7). + + Without this the app runs happily with a key anyone can read out of the + repository, which makes CSRF tokens forgeable and the session cookie + signable. render.yaml generates a key, but the Docker and self-hosted paths + in the README leave it to the operator. + """ + if not is_production(): + return + secret = app.config.get('SECRET_KEY') + if not secret or secret == DEV_SECRET_KEY: + raise RuntimeError( + 'SECRET_KEY must be set to a random value when APP_ENV=production; ' + 'the built-in development key is public.' + ) + + +# --- gzip (P1/D1) ----------------------------------------------------------- +# +# /process returns the full flattened dataset, so a 10 MB input commonly means a +# 5-20 MB response body. Repetitive JSON compresses 5-10x, which is the single +# largest transfer win available. Implemented in-repo rather than via +# Flask-Compress: ~40 lines against a new pinned dependency (D1). +# +# It does NOT reduce peak server memory or the client's parse cost -- the browser +# still receives, decompresses and stores the whole dataset. Those are P2/P5/P12. + +COMPRESSIBLE_MIMETYPES = frozenset( + { + 'application/json', + 'application/javascript', + 'application/xml', + 'image/svg+xml', + } +) + + +def _mark_varies_on_encoding(response): + """Add Accept-Encoding to Vary without duplicating an existing entry.""" + existing = [value.strip().lower() for value in response.headers.get('Vary', '').split(',')] + if 'accept-encoding' not in existing: + response.headers.add('Vary', 'Accept-Encoding') + + +def _is_compressible(response): + mimetype = (response.mimetype or '').lower() + return ( + mimetype.startswith('text/') + or mimetype.endswith('+json') + or (mimetype in COMPRESSIBLE_MIMETYPES) + ) + + +def compress_response(response): + """Gzip an eligible response body in place.""" + # A streamed or passthrough body must never be materialized here: reading it + # would consume the generator the export routes rely on. + if response.direct_passthrough or response.is_streamed: + return response + # 204/304 carry no body; HEAD must keep the headers a GET would produce, and + # rewriting Content-Length for a body we do not send would be wrong. + if response.status_code in (204, 304) or request.method == 'HEAD': + return response + if 'Content-Encoding' in response.headers: + return response + if not _is_compressible(response): + return response + + _mark_varies_on_encoding(response) + + # A substring test on the raw header treats `gzip;q=0` -- an explicit refusal + # -- as permission, because the token is present either way. Werkzeug's + # parsed Accept applies the q-values, so a zero quality reads as "not + # acceptable" and `*` reads as "anything", both per RFC 9110 12.5.3. + if request.accept_encodings.quality('gzip') <= 0: + return response + + data = response.get_data() + if len(data) < current_app.config.get('GZIP_MIN_SIZE', 1024): + return response + + compressed = gzip.compress(data, compresslevel=6) + if len(compressed) >= len(data): + return response + + # set_data recomputes Content-Length, so it always matches what we send. + response.set_data(compressed) + response.headers['Content-Encoding'] = 'gzip' + return response + + +# --- Rate-limit topology guard (2.10 / F12) --------------------------------- + + +def worker_count_from_start_command(argv): + """ + Return the worker count the start command names, or None. + + gunicorn forks its workers, so a worker inherits the master's argv. A bare + `--workers N` that disagrees with WEB_CONCURRENCY is exactly the drift this + exists to catch -- the app would validate one number while gunicorn ran + another. + """ + if not argv or 'gunicorn' not in os.path.basename(argv[0]): + return None + for index, arg in enumerate(argv): + if arg.startswith('--workers='): + value = arg.split('=', 1)[1] + elif arg in ('-w', '--workers') and index + 1 < len(argv): + value = argv[index + 1] + else: + continue + try: + return int(value) + except ValueError: + return None + return None + + +def worker_timeout_from_start_command(argv): + """ + Return the gunicorn worker timeout the start command names, or None. + + Same inheritance argument as worker_count_from_start_command: the worker + forked from the master sees the master's argv, so the command line is the + authoritative value rather than something the app has to be told twice. + """ + if not argv or 'gunicorn' not in os.path.basename(argv[0]): + return None + for index, arg in enumerate(argv): + if arg.startswith('--timeout='): + value = arg.split('=', 1)[1] + elif arg in ('-t', '--timeout') and index + 1 < len(argv): + value = argv[index + 1] + else: + continue + try: + return int(value) + except ValueError: + return None + return None + + +def check_fetch_timeout_headroom(app, argv=None): + """ + Refuse a deployment whose API fetch can outlive the worker that serves it. + + gunicorn kills a sync worker that has been silent for --timeout seconds. An + API_FETCH_TIMEOUT at or above that budget means requests.get() is still + waiting when the axe falls, so a slow upstream shows the user a 502 instead + of the timeout message the fetch path raises (P9). render.yaml's comment + already states the invariant; nothing enforced it. + + Raises under APP_ENV=production and warns otherwise, matching + check_rate_limit_topology: a local dev server has no gunicorn argv to read + and must not be blocked by a topology it does not have. + """ + argv = sys.argv if argv is None else argv + worker_timeout = worker_timeout_from_start_command(argv) + if worker_timeout is None: + return + + fetch_timeout = app.config.get('API_FETCH_TIMEOUT', 30) + if fetch_timeout < worker_timeout: + return + + message = ( + f'API_FETCH_TIMEOUT is {fetch_timeout}s but gunicorn runs ' + f'--timeout {worker_timeout}s, so a slow upstream fetch is killed with the ' + f'worker and the client sees a 502 rather than the fetch timeout; raise ' + f'--timeout above API_FETCH_TIMEOUT, or lower API_FETCH_TIMEOUT below it' + ) + if is_production(): + raise RuntimeError(message) + logger.warning(message) + + +def check_rate_limit_topology(app, argv=None): + """ + Refuse a production deployment whose rate limiting cannot be trusted. + + Raises under APP_ENV=production and warns otherwise, so local development is + never blocked by a topology it does not have. + """ + argv = sys.argv if argv is None else argv + production = is_production() + + workers = app.config.get('WEB_CONCURRENCY', 1) + replicas = app.config.get('APP_REPLICAS', 1) + storage = app.config.get('RATELIMIT_STORAGE_URI', 'memory://') + storage_is_shared = not storage.startswith('memory:') + + problems = [] + unverified = False + + if production: + if not app.config.get('WEB_CONCURRENCY_DECLARED'): + problems.append( + 'WEB_CONCURRENCY is not declared; under APP_ENV=production the worker ' + 'count must be explicit, because a default of 1 makes an undeclared ' + 'multi-worker deployment look single-worker' + ) + unverified = True + if not app.config.get('APP_REPLICAS_DECLARED'): + problems.append( + 'APP_REPLICAS is not declared; it must mirror the deployment layer ' + "(render.yaml's numInstances)" + ) + unverified = True + + commanded_workers = worker_count_from_start_command(argv) + if commanded_workers is not None and commanded_workers != workers: + problems.append( + f'the start command runs --workers {commanded_workers} but WEB_CONCURRENCY ' + f'is {workers}; derive the command from the variable ' + f'(gunicorn ... --workers "$WEB_CONCURRENCY") so the two cannot disagree' + ) + + if not storage_is_shared and (workers > 1 or replicas > 1 or unverified): + problems.append( + f'RATELIMIT_STORAGE_URI is {storage!r}, whose counters are process-local, ' + f'so the effective limit is multiplied by workers x replicas ' + f'({workers} x {replicas}); set a shared backend ' + '(RATELIMIT_STORAGE_URI=redis://...) or run one worker and one instance' + ) + + if not problems: + return + + message = 'Rate-limit topology is inconsistent: ' + '; '.join(problems) + if production: + raise RuntimeError(message) + logger.warning(message) + + +def _register_error_handlers(app): + """ + Keep every error response JSON (F10). + + Flask's built-in 413 and 500 pages are HTML, so app.js's response.json() + threw a SyntaxError on the body and surfaced a parse error instead of the + real problem. Every other error path in this app returns {"error": ...}. + """ + + @app.errorhandler(CSRFError) + def _csrf_error(_error): + # Flask-WTF's own 400 is an HTML page, and app.js calls response.json() + # on every /process reply -- so a missing token surfaced as a JSON parse + # error rather than "CSRF token missing" (F10). + return jsonify({'error': 'CSRF token missing or invalid'}), 400 + + @app.errorhandler(413) + def _request_entity_too_large(_error): + limit = app.config.get('MAX_CONTENT_LENGTH') or 0 + return jsonify({'error': f'Request too large (max {format_size(limit)})'}), 413 + + @app.errorhandler(500) + def _internal_server_error(_error): + return jsonify({'error': 'An internal error occurred'}), 500 + + @app.errorhandler(404) + def _not_found(_error): + return jsonify({'error': 'Not found'}), 404 + def create_app(config_class=Config): """Create and configure the Flask application.""" app = Flask(__name__) app.config.from_object(config_class) + _assert_production_secret_key(app) + check_rate_limit_topology(app) + check_fetch_timeout_headroom(app) + + if app.config.get('TRUST_PROXY'): + # Exactly one trusted hop. Behind Render's load balancer or an Nginx + # reverse proxy every request otherwise appears to come from the proxy + # IP, so all users share one rate-limit bucket and one client can exhaust + # the site's quota (F12). x_proto also makes request.is_secure correct, + # which the HSTS header (1.5) and the Secure cookie flag (1.14) rely on. + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1) + # Initialize extensions csrf.init_app(app) limiter.init_app(app) # Security headers on every response app.after_request(apply_security_headers) + app.after_request(compress_response) + + _register_error_handlers(app) # Register routes from routes import bp + app.register_blueprint(bp) return app diff --git a/config.py b/config.py index 38c34a1..32d3562 100644 --- a/config.py +++ b/config.py @@ -2,29 +2,193 @@ import os +# Publicly known, and therefore only ever acceptable outside production. +DEV_SECRET_KEY = 'dev-secret-key-change-in-production' + +# See MAX_EXPORT_CELLS below and docs/export-budget-v1.2.md for how this number +# was measured. +DEFAULT_MAX_EXPORT_CELLS = 250_000 + + +def is_production(): + """ + True when this process is running as a production deployment. + + `APP_ENV=production` is the single canonical signal, checked through this one + helper by the SECRET_KEY fail-fast (F7), the Secure cookie flag (F16) and the + rate-limit topology guard (2.10). + + Two things it deliberately is not: + + - It is never inferred from `not DEBUG`. The documented local run + `python app.py` has DEBUG False by default, so that would block ordinary + development startup. + - No second spelling (`PRODUCTION=true`, `ENV=prod`, ...) is accepted. Two + accepted names let a deployment satisfy one gate and silently miss another + -- e.g. passing the SECRET_KEY check while SESSION_COOKIE_SECURE stays off. + """ + return os.environ.get('APP_ENV', '').strip().lower() == 'production' + + +def env_int(name, default): + """ + Read an integer setting, failing with a message that names the variable. + + Plain int(os.environ.get(...)) raises a bare ValueError from deep inside the + import, which tells an operator nothing about which variable they mistyped + (F7). + """ + raw = os.environ.get(name) + if raw is None or raw.strip() == '': + return default + try: + return int(raw.strip()) + except ValueError: + raise RuntimeError(f'Environment variable {name} must be an integer, got {raw!r}') from None + + +def env_int_set(name, default): + """ + Read a comma-separated integer list (e.g. an allowlist of ports). + + Only an UNSET variable selects the default. An explicitly empty value yields + an empty set, which is how an operator disables a list-based check -- folding + the two together silently restored the default and made the documented + escape hatch a no-op. A blank element inside a non-empty list (`80,,443`, or + a lone `,`) is rejected rather than dropped, because dropping it can empty a + security allowlist without saying so. + """ + raw = os.environ.get(name) + if raw is None: + raw = default + if raw.strip() == '': + return frozenset() + parts = [part.strip() for part in raw.split(',')] + if not all(parts): + # `80,,443` and a lone `,` used to silently drop an element or empty the + # set entirely. security.validate_url reads an empty allowlist as + # "unrestricted", so a typo removed the outbound port restriction. + raise RuntimeError( + f'Environment variable {name} has an empty element, got {raw!r}; ' + f'use an empty value to disable the check' + ) + try: + return frozenset(int(part) for part in parts) + except ValueError: + raise RuntimeError( + f'Environment variable {name} must be a comma-separated list of integers, got {raw!r}' + ) from None + + +def env_positive_int(name, default): + """ + Read an integer setting that must be >= 1. + + ThreadPoolExecutor rejects max_workers <= 0, so without this an + API_DNS_MAX_WORKERS of 0 sailed through import and blew up as a 500 inside + the first API fetch instead of failing fast like every other misconfiguration. + """ + value = env_int(name, default) + if value < 1: + raise RuntimeError(f'Environment variable {name} must be >= 1, got {value}') + return value + class Config: """Flask configuration with env var overrides.""" - SECRET_KEY = os.environ.get('SECRET_KEY', 'dev-secret-key-change-in-production') + SECRET_KEY = os.environ.get('SECRET_KEY', DEV_SECRET_KEY) # Upload and payload limits - MAX_CONTENT_LENGTH = int(os.environ.get('MAX_UPLOAD_SIZE', 10 * 1024 * 1024)) + MAX_CONTENT_LENGTH = env_int('MAX_UPLOAD_SIZE', 10 * 1024 * 1024) # Preview and processing - PREVIEW_ROW_LIMIT = int(os.environ.get('PREVIEW_ROW_LIMIT', 25)) - API_FETCH_TIMEOUT = int(os.environ.get('API_FETCH_TIMEOUT', 30)) - API_FETCH_MAX_RESPONSE = int(os.environ.get('API_FETCH_MAX_RESPONSE', 10 * 1024 * 1024)) - FLATTEN_MAX_DEPTH = int(os.environ.get('FLATTEN_MAX_DEPTH', 10)) + PREVIEW_ROW_LIMIT = env_int('PREVIEW_ROW_LIMIT', 25) + API_FETCH_TIMEOUT = env_int('API_FETCH_TIMEOUT', 30) + API_FETCH_MAX_RESPONSE = env_int('API_FETCH_MAX_RESPONSE', 10 * 1024 * 1024) + FLATTEN_MAX_DEPTH = env_int('FLATTEN_MAX_DEPTH', 10) + + # DNS admission control for API fetch (F6.1/P7). API_DNS_TIMEOUT bounds how + # long a REQUEST waits, not how long the lookup runs -- getaddrinfo exposes no + # timeout and cannot be cancelled. API_DNS_MAX_WORKERS bounds concurrency, + # which is the actual worker-starvation fix. + API_DNS_TIMEOUT = env_int('API_DNS_TIMEOUT', 3) + API_DNS_MAX_WORKERS = env_positive_int('API_DNS_MAX_WORKERS', 4) + API_DNS_ADMISSION_TIMEOUT = env_int('API_DNS_ADMISSION_TIMEOUT', 1) + + # Ports the API-fetch feature may connect to (F6.2/D5). Only the IP was + # checked before, so http://public.example.com:22 or :6379 passed. An empty + # value disables the check. + API_ALLOWED_PORTS = env_int_set('API_ALLOWED_PORTS', '80,443,8443') + + # Trust X-Forwarded-* from exactly one proxy hop (F12/D3). Off by default: + # with it unset, behavior is identical to v1.1 and forged headers are ignored. + TRUST_PROXY = os.environ.get('TRUST_PROXY', '0').strip().lower() in ('1', 'true', 'yes') - # Rate limiting (Flask-Limiter reads RATELIMIT_* keys automatically) - RATELIMIT_STORAGE_URI = 'memory://' + # Rate limiting (Flask-Limiter reads RATELIMIT_* keys automatically). + # + # memory:// counters are PROCESS-LOCAL, so the effective limit is multiplied + # by workers x replicas -- not by workers alone. This was hardcoded before + # v1.2, so no deployment could configure shared storage at all (2.10a/F12). + RATELIMIT_STORAGE_URI = os.environ.get('RATELIMIT_STORAGE_URI', 'memory://') + + # WEB_CONCURRENCY is the SINGLE source of truth for the worker count: gunicorn + # reads it natively and every documented start command passes + # --workers "$WEB_CONCURRENCY", so the number this process validates cannot + # drift from the number gunicorn actually runs. Replica count is invisible from + # inside the process, so APP_REPLICAS is a deployment-layer declaration that + # must mirror render.yaml's numInstances. + # + # Defaults of 1 fail OPEN -- an undeclared 4-worker deployment reads as + # single-worker -- so under APP_ENV=production both must be declared + # explicitly; see check_rate_limit_topology in app.py. + WEB_CONCURRENCY = env_int('WEB_CONCURRENCY', 1) + APP_REPLICAS = env_int('APP_REPLICAS', 1) + WEB_CONCURRENCY_DECLARED = (os.environ.get('WEB_CONCURRENCY') or '').strip() != '' + APP_REPLICAS_DECLARED = (os.environ.get('APP_REPLICAS') or '').strip() != '' RATELIMIT_DEFAULT = os.environ.get('RATE_LIMIT_DEFAULT', '120/minute') RATE_LIMIT_PROCESS = os.environ.get('RATE_LIMIT_PROCESS', '30/minute') RATE_LIMIT_EXPORT = os.environ.get('RATE_LIMIT_EXPORT', '60/minute') + # Cookie hardening (F16). Flask's defaults are HttpOnly=True but emit no + # SameSite attribute and never set Secure. Secure is tied to the explicit + # production signal: a plain local run has DEBUG False, so gating on + # `not DEBUG` would send Secure cookies over http and break CSRF-protected + # POSTs during development. + SESSION_COOKIE_HTTPONLY = True + SESSION_COOKIE_SAMESITE = 'Lax' + SESSION_COOKIE_SECURE = is_production() + + # XLSX-only export budget (P3/D6), in CELLS (rows x columns) because that is + # what drives openpyxl's memory -- a 10 MiB body with 3 columns and one with + # 500 columns have wildly different footprints at the same row count. + # + # Enabled by default: an unlimited default would leave P3 (High) unmitigated. + # The value is derived from the Performance Review section 4 measurement (see + # docs/export-budget-v1.2.md), not chosen by feel -- re-derive it whenever that + # measurement is re-run. 0 disables the guard for operators who knowingly opt + # out. CSV/TSV stay uncapped and streamed, so every dataset /process accepts + # remains exportable by some route. + MAX_EXPORT_CELLS = env_int('MAX_EXPORT_CELLS', DEFAULT_MAX_EXPORT_CELLS) + + # Static assets are revalidated on every navigation without this, costing a + # round trip per page load -- worst on a Render free-tier cold start (P6). + # Safe to cache for a day because the asset URLs carry ?v=APP_VERSION. + SEND_FILE_MAX_AGE_DEFAULT = env_int('STATIC_MAX_AGE', 86400) + + # Responses smaller than this are not worth a gzip round trip (P1). + GZIP_MIN_SIZE = env_int('GZIP_MIN_SIZE', 1024) + # Application metadata - APP_VERSION = '1.1.0' + APP_VERSION = '1.2.0' + + # F15: /health returns `version` by default (the existing contract). Operators + # who would rather not advertise it can set HEALTH_REVEAL_VERSION=0. + HEALTH_REVEAL_VERSION = os.environ.get('HEALTH_REVEAL_VERSION', '1').strip().lower() not in ( + '0', + 'false', + 'no', + ) # Debug mode DEBUG = os.environ.get('FLASK_DEBUG', '0').lower() in ('1', 'true', 'yes') diff --git a/docs/export-budget-v1.2.md b/docs/export-budget-v1.2.md new file mode 100644 index 0000000..3fbfec8 --- /dev/null +++ b/docs/export-budget-v1.2.md @@ -0,0 +1,113 @@ +# XLSX export budget — how `MAX_EXPORT_CELLS` was measured + +**Date:** 2026-08-21 +**Applies to:** roadmap task 2.3 / D6, performance finding P3. +**Result:** `DEFAULT_MAX_EXPORT_CELLS = 250_000` cells (`config.py`). + +D6 requires the guard's default to come from the Performance Review §4 +measurement rather than being chosen by feel. This file records the numbers it +was derived from, so it can be re-derived when the measurement is re-run. + +## Method + +Performance Review §4's protocol, with one documented deviation. + +Followed as written: + +| Parameter | This run | +|---|---| +| Units | `ru_maxrss` read on Linux (KiB), multiplied by 1024, reported in MiB | +| Peak | `resource.getrusage(RUSAGE_SELF).ru_maxrss`, read after the response was fully delivered | +| No warm-up in a measured process | Each measured process serves **exactly one** request, then exits | +| Two runs, not two samples | `baseline` = a freshly booted process that served **zero** requests; `measured` = a freshly booted process that served exactly one. `delta = measured − baseline` | +| Concurrency | 1 | +| Absolute and delta | Both recorded | +| Blocked pairs | None occurred; none were discarded | + +**Deviation:** requests were issued through the Flask test client inside a fresh +Python process rather than through a fresh gunicorn worker, and the sizing sweep +used one run-pair per shape rather than the median of three. The property the +protocol exists to protect — that `ru_maxrss` is monotonic per process and so +cannot be reset mid-process — is preserved: every measured number comes from a +process that served exactly one request. Numbers produced this way are +comparable to each other but should not be quoted against the §4 budget as if +they had come from the full gunicorn/median-of-three protocol. + +`MAX_CONTENT_LENGTH` was raised for the measurement only, so request-body size +would not mask the workbook cost. + +## Data + +| rows × cols | cells | delta (MiB) | absolute (MiB) | +|---|---|---|---| +| 15,000 × 10 | 150,000 | 73.6 | 114.3 | +| 83,333 × 3 | 249,999 | **138.9** | 179.6 | +| 50,000 × 3 | 150,000 | **84.3** | 125.0 | +| 20,000 × 10 | 200,000 | 98.6 | 139.2 | +| 2,000 × 150 | 300,000 | 137.3 | 178.0 | +| 91,666 × 3 | 274,998 | **152.2** | 193.0 | +| 100,000 × 3 | 300,000 | **161.7** | 202.4 | +| 40,000 × 10 | 400,000 | 195.6 | 236.3 | + +At equal cell counts the **narrow, tall** shape is consistently the most +expensive (84.3 vs 73.6 MiB at 150,000 cells; 161.7 vs 137.3 at 300,000) — +openpyxl carries per-row overhead on top of per-cell. Three columns is therefore +the worst aspect ratio tested and the one the budget is sized against. + +This is also why the budget is expressed in **cells** rather than rows: 50,000 +rows costs 84.3 MiB at 3 columns and 40,000 rows costs 195.6 MiB at 10, so a row +count says almost nothing about the footprint on its own. + +## Derivation + +Fitting the two bracketing points of the 3-column series: + +```text +(150,000 cells, 84.3 MiB) and (274,998 cells, 152.2 MiB) +slope = 0.000543 MiB/cell +intercept = 2.8 MiB +150 MiB crossing = (150 − 2.8) / 0.000543 ≈ 270,900 cells +``` + +The 274,998-cell run measured **152.2 MiB — over the 150 MiB target**, so the +crossing is real and not an artifact of extrapolation. The budget is set at +**250,000 cells**, roughly 8% below the crossing, which is the margin +run-to-run variance on these measurements needs. + +That value was then measured directly rather than left as an extrapolation: +83,333 × 3 = 249,999 cells came back at **138.9 MiB delta / 179.6 MiB absolute** +(the fit predicted 138.6). It passes both halves of the §4 verdict — under the +150 MiB delta target, and well under the 256 MiB absolute ceiling for a 512 MiB +container. + +## What the budget does and does not cover + +- It is **XLSX-only**. CSV and TSV are generator-streamed and stay uncapped, so + every dataset `/process` accepts remains exportable by some route. That, not + an unbounded XLSX path, is what keeps the export contract as wide as the input + contract. +- It is **enabled by default**. An unlimited default would leave P3 (High) + unmitigated. `MAX_EXPORT_CELLS=0` disables it for operators who knowingly opt + out. +- It is **advertised, never silent**. `/process` returns `total_cells` and + `max_export_cells` so the client greys the Excel entry out before the user + clicks; `/export-xlsx` independently returns 400 for direct API callers. There + is no truncation — a partial spreadsheet is worse than a refusal. +- The 10 MiB request cap makes memory finite but not usefully bounded: the + JSON → Python → openpyxl → zip expansion multiplier is large and + data-dependent. The measured cell budget is the bound; the request cap is not + a substitute for it. + +## Reproducing + +The harness is not committed (it is a throwaway measurement script, and the +roadmap keeps perf tests out of CI). It does two things: + +1. `baseline`: boot `create_app()`, serve nothing, print `ru_maxrss`. +2. `measured`: boot `create_app()`, POST one synthetic `csv_data` body of + `rows × cols` cells to `/export-xlsx` with `MAX_EXPORT_CELLS=0`, assert 200, + then print `ru_maxrss`. + +Run each in its own process and subtract. Re-derive the fit above from at least +two points on the narrowest aspect ratio you care about, and re-run the +confirming point at the value you intend to ship. diff --git a/extensions.py b/extensions.py index bd1e0fa..afa6217 100644 --- a/extensions.py +++ b/extensions.py @@ -1,8 +1,21 @@ """Flask extensions (initialized without app, bound later via init_app).""" -from flask_wtf.csrf import CSRFProtect +from flask import request from flask_limiter import Limiter -from flask_limiter.util import get_remote_address +from flask_wtf.csrf import CSRFProtect + + +def client_ip_key(): + """ + Rate-limit bucket key: the client's IP. + + Deliberately reads request.remote_addr rather than the X-Forwarded-For + header. remote_addr is only rewritten from that header when ProxyFix is + installed, which create_app does exclusively under TRUST_PROXY=1 (F12/D3). + Reading the raw header here would let any client forge its own bucket. + """ + return request.remote_addr or 'unknown' + csrf = CSRFProtect() -limiter = Limiter(key_func=get_remote_address) +limiter = Limiter(key_func=client_ip_key) diff --git a/helpers.py b/helpers.py index cf37459..7250133 100644 --- a/helpers.py +++ b/helpers.py @@ -1,9 +1,16 @@ """Data processing helpers for JSON flattening and table extraction.""" import json +from typing import Any -def flatten_for_csv(data, parent_key='', sep='.', _depth=0, max_depth=10): +def flatten_for_csv( + data: Any, + parent_key: str = '', + sep: str = '.', + _depth: int = 0, + max_depth: int = 10, +) -> dict[str, Any]: """ Flatten nested dictionaries for CSV export. Arrays are converted to JSON strings. @@ -17,10 +24,12 @@ def flatten_for_csv(data, parent_key='', sep='.', _depth=0, max_depth=10): items = [] if isinstance(data, dict): for k, v in data.items(): - new_key = f"{parent_key}{sep}{k}" if parent_key else k + new_key = f'{parent_key}{sep}{k}' if parent_key else k if isinstance(v, dict): items.extend( - flatten_for_csv(v, new_key, sep=sep, _depth=_depth + 1, max_depth=max_depth).items() + flatten_for_csv( + v, new_key, sep=sep, _depth=_depth + 1, max_depth=max_depth + ).items() ) elif isinstance(v, list): items.append((new_key, json.dumps(v))) @@ -31,29 +40,97 @@ def flatten_for_csv(data, parent_key='', sep='.', _depth=0, max_depth=10): return dict(items) -def extract_table_data(json_data): +# Characters that make a spreadsheet treat a cell as a formula (or let it smuggle +# extra rows/fields past a delimited parser). OWASP lists all seven. +FORMULA_TRIGGERS = ('=', '+', '-', '@', '\t', '\r', '\n') + + +def serialize_cell_value(value: Any) -> Any: + """ + Reduce one cell value to the scalar an export writer can emit. + + Containers become their JSON text; everything else is passed through + unchanged so numbers stay numbers in the workbook. + """ + if isinstance(value, (dict, list)): + return json.dumps(value) + return value + + +def is_formula_trigger(value: Any) -> bool: + """True when a serialized value would be read as a formula by a spreadsheet.""" + return isinstance(value, str) and value.startswith(FORMULA_TRIGGERS) + + +def sanitize_cell(value: Any) -> Any: + """ + Serialize a cell for the delimited formats (CSV/TSV) and defuse formula + injection (CWE-1236). + + Delimited output has no type channel, so a dangerous value is prefixed with a + single quote -- the OWASP mitigation for CSV. XLSX does not use this: it has a + real string type, so `export_xlsx` writes the untouched value and pins the + cell's data_type instead (see `is_formula_trigger`). Lossless exports (JSONL) + and Markdown must not call this. + """ + serialized = serialize_cell_value(value) + if is_formula_trigger(serialized): + return "'" + serialized + return serialized + + +def flatten_rows(rows: list[Any], max_depth: int = 10) -> tuple[list[dict[str, Any]], list[str]]: + """ + Flatten every row and collect the column names in a single pass. + + Returns (flattened_rows, sorted_column_names). Previously the caller + flattened, then walked the result again with get_all_columns -- two full + passes over the largest structure in the request (P8). + + The names are accumulated in a set and sorted once at the end, which is + exactly what get_all_columns produces; set iteration order is not relied on. + """ + flattened = [] + columns = set() + for row in rows: + flat = flatten_for_csv(row, max_depth=max_depth) + flattened.append(flat) + if isinstance(flat, dict): + columns.update(flat.keys()) + return flattened, sorted(columns) + + +def extract_table_data(json_data: Any, _depth: int = 0, max_depth: int = 10) -> list[Any]: """ Extract tabular data from JSON. Handles arrays of objects, nested arrays, and single objects. Returns a list of row dicts. + + Mirrors flatten_for_csv's depth cap (F8): a payload nested a thousand levels + deep is valid JSON, and without the cap the descent into nested dicts blows + the Python stack and turns a client-supplied document into a 500. At the cap + the remaining structure becomes a single row rather than being explored. """ + if _depth >= max_depth: + return [json_data] if isinstance(json_data, dict) else [] + if isinstance(json_data, list): if len(json_data) > 0 and isinstance(json_data[0], dict): return json_data else: - return [{"value": item} for item in json_data] + return [{'value': item} for item in json_data] if isinstance(json_data, dict): - for key, value in json_data.items(): + for value in json_data.values(): if isinstance(value, list) and len(value) > 0: if isinstance(value[0], dict): return value else: - return [{"value": item} for item in value] + return [{'value': item} for item in value] - for key, value in json_data.items(): + for value in json_data.values(): if isinstance(value, dict): - result = extract_table_data(value) + result = extract_table_data(value, _depth=_depth + 1, max_depth=max_depth) if result: return result @@ -62,7 +139,7 @@ def extract_table_data(json_data): return [] -def parse_jsonl(text): +def parse_jsonl(text: str) -> list[Any]: """ Parse JSONL (JSON Lines) text into a list of objects. Each non-empty line is parsed as a separate JSON value. @@ -75,43 +152,11 @@ def parse_jsonl(text): try: rows.append(json.loads(line)) except json.JSONDecodeError as e: - raise ValueError(f'Invalid JSON on line {i}: {str(e)}') + raise ValueError(f'Invalid JSON on line {i}: {e}') from e return rows -def find_candidate_arrays(json_data, prefix='', candidates=None): - """ - Find all arrays of objects in JSON data, returning their paths and metadata. - Used when multiple arrays exist so the user can choose which to tabularize. - """ - if candidates is None: - candidates = [] - - if isinstance(json_data, list): - if len(json_data) > 0 and isinstance(json_data[0], dict): - sample_keys = sorted(json_data[0].keys())[:5] - candidates.append({ - 'path': prefix or '(root)', - 'length': len(json_data), - 'sample_keys': sample_keys - }) - elif isinstance(json_data, dict): - for key, value in json_data.items(): - path = f'{prefix}.{key}' if prefix else key - if isinstance(value, list) and len(value) > 0 and isinstance(value[0], dict): - sample_keys = sorted(value[0].keys())[:5] - candidates.append({ - 'path': path, - 'length': len(value), - 'sample_keys': sample_keys - }) - elif isinstance(value, dict): - find_candidate_arrays(value, path, candidates) - - return candidates - - -def extract_by_path(json_data, path): +def extract_by_path(json_data: Any, path: str) -> Any: """ Extract data from JSON using a dot-notation path. Numeric parts traverse arrays (e.g. 'data.0.orders'). @@ -138,10 +183,97 @@ def extract_by_path(json_data, path): return current -def get_all_columns(data): +def get_all_columns(data: list[Any]) -> list[str]: """Get all unique column names from the data, sorted alphabetically.""" columns = set() for row in data: if isinstance(row, dict): columns.update(row.keys()) - return sorted(list(columns)) + return sorted(columns) + + +# --- Preview projection (P2.2/P5) ------------------------------------------- +# +# `preview` rows carry full-fidelity nested structures, so a 50k-key object or a +# 5 MB string cell is handed straight to the browser and freezes the tab. These +# caps apply to the PREVIEW ONLY: the projection is a copy, so table_data and +# csv_data -- and therefore every export -- keep the original values. + +PREVIEW_MAX_STRING = 256 +PREVIEW_MAX_ITEMS = 20 +PREVIEW_TRUNCATION_SUFFIX = '… (truncated)' + + +def _truncate_preview_value( + value: Any, max_string: int, max_items: int, depth: int, max_depth: int +) -> Any: + """Return a capped copy of one nested value.""" + if isinstance(value, str): + if len(value) > max_string: + return value[:max_string] + PREVIEW_TRUNCATION_SUFFIX + return value + + if isinstance(value, dict): + if depth >= max_depth: + return PREVIEW_TRUNCATION_SUFFIX + truncated = {} + for index, (key, item) in enumerate(value.items()): + if index >= max_items: + truncated[PREVIEW_TRUNCATION_SUFFIX] = f'… and {len(value) - max_items} more keys' + break + truncated[key] = _truncate_preview_value( + item, max_string, max_items, depth + 1, max_depth + ) + return truncated + + if isinstance(value, list): + if depth >= max_depth: + return PREVIEW_TRUNCATION_SUFFIX + truncated = [ + _truncate_preview_value(item, max_string, max_items, depth + 1, max_depth) + for item in value[:max_items] + ] + if len(value) > max_items: + truncated.append(f'… and {len(value) - max_items} more items') + return truncated + + return value + + +def preview_truncate( + row: Any, + max_string: int = PREVIEW_MAX_STRING, + max_items: int = PREVIEW_MAX_ITEMS, + max_depth: int = 10, +) -> Any: + """ + Build a capped COPY of one preview row. + + Every column of the row survives -- dropping columns would make the preview + table disagree with its own header. Only the values inside are capped. + Nothing is mutated: exports read the original rows. + """ + if not isinstance(row, dict): + return _truncate_preview_value(row, max_string, max_items, 0, max_depth) + return { + key: _truncate_preview_value(value, max_string, max_items, 1, max_depth) + for key, value in row.items() + } + + +def format_size(num_bytes: int) -> str: + """ + Render a byte count in the largest unit that stays non-zero. + + Integer-dividing by a MiB reported "max 0MB" for any limit below 1 MiB, + which tells an operator nothing about what they configured. + + Lives here rather than in app.py because both the 413 handler and the API + size cap in routes.py need it: importing it from app.py made `import routes` + pull in app.py, which imports `bp` back out of the still-initializing + routes module (ImportError on any routes-first import). + """ + for unit, size in (('MB', 1024 * 1024), ('KB', 1024)): + if num_bytes >= size: + return f'{num_bytes // size}{unit}' + return f'{num_bytes} bytes' diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..6e9332e --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,26 @@ +[tool.ruff] +target-version = "py311" +line-length = 100 +exclude = [".venv", "venv", "__pycache__"] +# Only lint/format Python sources; prose files carry illustrative snippets that +# must stay byte-identical to what they document. +include = ["*.py", "*.pyi"] + +[tool.ruff.lint] +select = ["E", "F", "W", "I", "UP", "B", "C4", "SIM"] +ignore = [ + # Long URLs and prose in docstrings/comments are allowed; code lines are still + # held to line-length by the formatter. + "E501", +] + +[tool.ruff.lint.per-file-ignores] +"tests/*" = ["B011"] + +[tool.ruff.format] +quote-style = "single" + +[tool.pytest.ini_options] +testpaths = ["tests"] +python_files = ["test_*.py"] +addopts = "-ra" diff --git a/render.yaml b/render.yaml index 3d06f07..e7fc2c3 100644 --- a/render.yaml +++ b/render.yaml @@ -6,14 +6,40 @@ services: name: json-table-converter runtime: python buildCommand: pip install -r requirements.txt - startCommand: gunicorn "app:create_app()" --bind 0.0.0.0:$PORT + # --workers comes from WEB_CONCURRENCY so the number gunicorn runs is the + # same number the app validates (roadmap 2.10b). --timeout must stay above + # API_FETCH_TIMEOUT (default 30s) or a slow API fetch races the worker kill + # and the client sees a 502 instead of the timeout message (P9). That is no + # longer only a comment: check_fetch_timeout_headroom reads this --timeout + # out of gunicorn's argv at startup and refuses to boot in production if + # API_FETCH_TIMEOUT has caught up with it. + startCommand: >- + gunicorn "app:create_app()" + --bind 0.0.0.0:$PORT + --workers "$WEB_CONCURRENCY" + --timeout 60 envVars: - key: PYTHON_VERSION value: 3.14.5 - key: SECRET_KEY generateValue: true + # The one canonical production signal: enables the SECRET_KEY fail-fast + # (F7), the Secure session cookie (F16) and the topology guard (2.10). + - key: APP_ENV + value: production + # Rate-limit topology. memory:// counters are process-local, so the + # effective limit is multiplied by workers x replicas. Going above 1x1 + # requires a shared RATELIMIT_STORAGE_URI (redis://...) plus + # requirements-redis.txt in the build command -- the guard refuses to start + # otherwise. APP_REPLICAS must mirror numInstances below. + - key: WEB_CONCURRENCY + value: 1 + - key: APP_REPLICAS + value: 1 # Free tier settings plan: free + numInstances: 1 # No persistent disk = no data storage - # Auto-deploy on push - autoDeploy: true + # Auto-deploy on push, but only once CI checks pass. Render blocks the + # deploy when the commit has failing checks or no checks at all. + autoDeployTrigger: checksPass diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..c6dcc32 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,12 @@ +# Development / CI dependencies. Production installs requirements.txt only. +-r requirements.txt + +# The optional Redis client, so the test suite can exercise the documented +# multi-worker configuration (roadmap 2.10d). Production installs it separately +# via requirements-redis.txt; it is deliberately not in requirements.txt. +-r requirements-redis.txt + +pytest==9.0.3 +ruff==0.16.4 +coverage==7.15.4 +pip-audit==2.10.1 diff --git a/requirements-redis.txt b/requirements-redis.txt new file mode 100644 index 0000000..0582d07 --- /dev/null +++ b/requirements-redis.txt @@ -0,0 +1,10 @@ +# Optional: the client Flask-Limiter needs for a shared RATELIMIT_STORAGE_URI. +# +# Install this ONLY when running more than one gunicorn worker or more than one +# replica, i.e. when RATELIMIT_STORAGE_URI is set to redis://... The default +# single-worker / single-instance deployment uses memory:// and does not need it. +# +# pip install -r requirements.txt -r requirements-redis.txt +-r requirements.txt + +redis==8.1.0 diff --git a/requirements.txt b/requirements.txt index 3dc4d49..c30a028 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,6 @@ -Flask==3.0.0 -requests==2.31.0 -gunicorn==21.2.0 +Flask==3.1.3 +requests==2.33.0 +gunicorn==22.0.0 Flask-WTF==1.2.1 Flask-Limiter==3.5.0 -pytest==7.4.4 -openpyxl==3.1.2 +openpyxl==3.1.5 diff --git a/routes.py b/routes.py index 65e685c..25ee93e 100644 --- a/routes.py +++ b/routes.py @@ -1,38 +1,396 @@ """Flask route handlers.""" -import json import csv import io +import json import logging +import re + import requests +from flask import ( + Blueprint, + Response, + current_app, + jsonify, + render_template, + request, + send_file, +) +from openpyxl import Workbook from requests.auth import HTTPBasicAuth -from flask import Blueprint, render_template, request, jsonify, Response, current_app +from werkzeug.exceptions import HTTPException +from config import DEFAULT_MAX_EXPORT_CELLS from extensions import limiter -from security import validate_url from helpers import ( - flatten_for_csv, extract_table_data, get_all_columns, parse_jsonl, - extract_by_path + extract_by_path, + extract_table_data, + flatten_rows, + format_size, + get_all_columns, + is_formula_trigger, + parse_jsonl, + preview_truncate, + sanitize_cell, + serialize_cell_value, ) +from security import validate_url logger = logging.getLogger(__name__) bp = Blueprint('main', __name__) +# F4: outbound header names come from the client, so this is a real allowlist, +# not a token regex plus a list of names to reject. A denylist cannot work here: +# HTTP field names are case-insensitive, so `Host`, `PROXY-AUTHORIZATION` and +# `CoNnEcTiOn` all slip past a lowercase membership test, and anything simply +# absent from the list would pass. Names are stripped and lowercased before the +# membership test; the regex stays only as a syntax check on top of it. +ALLOWED_OUTBOUND_HEADERS = frozenset( + { + 'accept', + 'accept-language', + 'authorization', + 'user-agent', + 'x-api-key', + } +) + +HEADER_NAME_PATTERN = re.compile(r'^[A-Za-z0-9-]+$') + +# F13: `accept=".json,.jsonl"` on the file input is client-side only. The +# extension check is the authoritative one; the content-type check is deliberately +# lenient because browsers send application/octet-stream (or nothing at all) for +# extensions they do not recognize -- .jsonl in particular -- so a strict list +# would reject legitimate uploads. +ALLOWED_UPLOAD_EXTENSIONS = ('.json', '.jsonl') + +# Rows buffered before a chunk of CSV is handed to the WSGI server. +CSV_STREAM_CHUNK_ROWS = 500 + +ALLOWED_UPLOAD_CONTENT_TYPES = frozenset( + { + '', + 'application/json', + 'application/jsonl', + 'application/ld+json', + 'application/octet-stream', + 'application/x-ndjson', + 'text/json', + 'text/plain', + 'text/x-json', + } +) + + +def validate_upload(file_storage): + """Return an error message for a file we will not try to parse, else None.""" + filename = (file_storage.filename or '').strip().lower() + if not filename.endswith(ALLOWED_UPLOAD_EXTENSIONS): + return 'File must be a .json or .jsonl file' + + content_type = (file_storage.mimetype or '').strip().lower() + if content_type not in ALLOWED_UPLOAD_CONTENT_TYPES: + return f'Unsupported content type: {content_type}' + + return None + + +def is_allowed_outbound_header(name): + """True when a client-supplied outbound header name may be forwarded.""" + normalized = name.strip().lower() + if not HEADER_NAME_PATTERN.match(normalized): + return False + return normalized in ALLOWED_OUTBOUND_HEADERS + + @bp.route('/') def index(): """Render the main page.""" return render_template('index.html') +def _health_payload(status='ok', **extra): + payload = {'status': status} + if current_app.config.get('HEALTH_REVEAL_VERSION', True): + payload['version'] = current_app.config['APP_VERSION'] + payload.update(extra) + return payload + + @bp.route('/health') def health(): - """Health check endpoint.""" - return jsonify({ - 'status': 'ok', - 'version': current_app.config['APP_VERSION'] - }) + """Health check endpoint (unchanged contract).""" + return jsonify(_health_payload()) + + +@bp.route('/health/live') +def health_live(): + """ + Liveness: is the process up at all? + + Deliberately does no dependency work, so a restart loop caused by a failing + dependency check is impossible. + """ + return jsonify(_health_payload()) + + +@bp.route('/health/ready') +def health_ready(): + """ + Readiness: is this process able to serve traffic? + + Checks only what is cheap and local -- that the rate limiter has usable + storage. Returns 503 when it cannot serve, so a load balancer takes it out + of rotation rather than sending it requests. There is deliberately no Excel + writer check; see the comment below. + """ + checks = {} + + storage_uri = current_app.config.get('RATELIMIT_STORAGE_URI', 'memory://') + try: + # limits' storages report an unreachable backend by RETURNING False -- + # RedisStorage.check() swallows the connection error and returns False -- + # so discarding the result would mark a dead Redis as healthy and keep a + # load balancer sending traffic to an instance that cannot rate-limit. + storage_healthy = bool(limiter.storage.check()) + except Exception: + # The URI is config, not payload, but keep the reason out of the body. + logger.warning('Rate-limit storage check failed') + storage_healthy = False + checks['rate_limit_storage'] = 'ok' if storage_healthy else 'unavailable' + + # No xlsx_writer check: openpyxl is imported at module scope (task 3.4), so a + # missing dependency stops routes.py from importing at all and this handler + # could never run to report it. A check that cannot fail is noise. + + ready = all(value == 'ok' for value in checks.values()) + payload = _health_payload( + status='ok' if ready else 'degraded', + checks=checks, + rate_limit_storage_backend=storage_uri.split(':', 1)[0], + ) + return jsonify(payload), (200 if ready else 503) + + +def _build_api_auth(auth_method): + """ + Build the outbound headers/auth/params for one auth method. + + Returns (headers, auth, params, error_response). error_response is None + unless the client asked for something we refuse to forward. + """ + headers = {} + auth = None + params = {} + + if auth_method == 'api_key': + header_name = request.form.get('api_key_header', 'X-API-Key') + api_key = request.form.get('api_key', '') + if api_key: + if not is_allowed_outbound_header(header_name): + return ( + None, + None, + None, + ( + jsonify( + { + 'error': 'Header name is not permitted. Allowed: ' + + ', '.join(sorted(ALLOWED_OUTBOUND_HEADERS)) + } + ), + 400, + ), + ) + headers[header_name.strip()] = api_key + elif auth_method == 'basic': + username = request.form.get('basic_username', '') + password = request.form.get('basic_password', '') + if username: + auth = HTTPBasicAuth(username, password) + elif auth_method == 'bearer': + bearer_token = request.form.get('bearer_token', '') + if bearer_token: + headers['Authorization'] = f'Bearer {bearer_token}' + elif auth_method == 'query_param': + param_name = request.form.get('query_param_name', 'api_key') + param_value = request.form.get('query_param_value', '') + if param_value: + params[param_name] = param_value + + return headers, auth, params, None + + +def _parse_payload(text, data_format): + """Parse a payload as JSON or JSONL according to the requested format.""" + return parse_jsonl(text) if data_format == 'jsonl' else json.loads(text) + + +def _load_from_file(data_format): + """Read and parse an uploaded file. Returns (data, error_response).""" + if 'json_file' not in request.files: + return None, (jsonify({'error': 'No file uploaded'}), 400) + + file = request.files['json_file'] + if file.filename == '': + return None, (jsonify({'error': 'No file selected'}), 400) + + upload_error = validate_upload(file) + if upload_error: + return None, (jsonify({'error': upload_error}), 400) + + try: + content = file.read().decode('utf-8') + return _parse_payload(content, data_format), None + except UnicodeDecodeError: + return None, (jsonify({'error': 'File must be UTF-8 encoded'}), 400) + except (json.JSONDecodeError, ValueError) as e: + return None, (jsonify({'error': f'Invalid data in file: {e}'}), 400) + + +def _load_from_paste(data_format): + """Parse pasted text. Returns (data, error_response).""" + pasted_json = request.form.get('pasted_json', '').strip() + if not pasted_json: + return None, (jsonify({'error': 'No JSON provided'}), 400) + + try: + return _parse_payload(pasted_json, data_format), None + except (json.JSONDecodeError, ValueError) as e: + return None, (jsonify({'error': f'Invalid data: {e}'}), 400) + + +def _load_from_api(data_format): + """Fetch and parse a remote payload. Returns (data, error_response).""" + api_url = request.form.get('api_url', '').strip() + if not api_url: + return None, (jsonify({'error': 'No API URL provided'}), 400) + + is_valid, error_msg = validate_url(api_url) + if not is_valid: + return None, (jsonify({'error': error_msg}), 400) + + headers, auth, params, error_response = _build_api_auth(request.form.get('auth_method', 'none')) + if error_response: + return None, error_response + + try: + timeout = current_app.config['API_FETCH_TIMEOUT'] + max_size = current_app.config['API_FETCH_MAX_RESPONSE'] + + # Use original URL to preserve TLS/SNI verification. + # SSRF mitigated by: pre-request DNS validation + disabled redirects. + # Residual DNS rebinding risk is minimal (requires attacker-controlled + # DNS with sub-millisecond TTL between our check and requests' connect). + # Context-managed: with stream=True the socket stays open until the body + # is consumed or the response is closed, and the size-limit path below + # returns with the body only partly read. Without this the connection is + # never returned to the pool and leaks until garbage collection -- which + # any caller can trigger repeatedly by pointing /process at a large + # endpoint. + with requests.get( + api_url, + headers=headers, + auth=auth, + params=params, + timeout=timeout, + stream=True, + allow_redirects=False, + ) as resp: + resp.raise_for_status() + + content = bytearray() + for chunk in resp.iter_content(chunk_size=8192): + content.extend(chunk) + if len(content) > max_size: + return None, ( + jsonify( + { + 'error': f'API response exceeds maximum size ({format_size(max_size)})' + } + ), + 400, + ) + + # bytearray decodes directly; bytes(content) made a second full copy + # of the response body at peak (P12). Parsing, flattening and jsonify + # still materialize the dataset -- this removes one copy, it does not + # make the pipeline low-memory. + return _parse_payload(content.decode('utf-8'), data_format), None + + except requests.exceptions.Timeout: + return None, (jsonify({'error': 'API request timed out'}), 400) + except requests.exceptions.RequestException: + # Fixed message, no interpolation: requests' exception text carries the + # full URL, and the query string, fragment, userinfo AND path can each + # hold a token (F3/F9). Redacting one component is not enough, so nothing + # user-controlled is logged at all. + logger.warning('API request failed') + return None, (jsonify({'error': 'API request failed'}), 400) + except UnicodeDecodeError: + # A subclass of ValueError, so it has to be caught first -- otherwise a + # binary or latin-1 response was reported as malformed JSONL, which + # sends the caller looking at the wrong thing (and is simply untrue for + # a JSON request). + return None, (jsonify({'error': 'API response must be UTF-8 encoded'}), 400) + except json.JSONDecodeError: + return None, (jsonify({'error': 'API response is not valid JSON'}), 400) + except ValueError: + # parse_jsonl raises ValueError on a malformed line. Without this it + # reached the outer handler as a 500 with a logged traceback, although it + # is the caller's data that is wrong (F9). + return None, (jsonify({'error': 'API response is not valid JSONL'}), 400) + + +def _load_input(data_format): + """ + Dispatch to the requested input method. Returns (data, error_response). + + Exactly one of the two is meaningful: a non-None error_response is the + caller's return value. + """ + loaders = { + 'file': _load_from_file, + 'paste': _load_from_paste, + 'api': _load_from_api, + } + loader = loaders.get(request.form.get('input_method')) + if loader is None: + return None, (jsonify({'error': 'Invalid input method'}), 400) + return loader(data_format) + + +def _select_table_data(json_data, json_path): + """ + Turn the chosen JSON path into table rows. Returns (rows, error_response). + + With no path the client has not chosen a node yet, so the whole document + goes back for the tree picker -- that 200 is a response, not an error, and + rides in the error_response slot because it is equally terminal. + """ + if not json_path: + return None, (jsonify({'needs_selection': True, 'raw_json': json_data}), 200) + + selected = extract_by_path(json_data, json_path) + if selected is None: + return None, (jsonify({'error': f'Path "{json_path}" not found'}), 400) + + if isinstance(selected, list): + rows = extract_table_data(selected, max_depth=current_app.config['FLATTEN_MAX_DEPTH']) + elif isinstance(selected, dict): + rows = [selected] + else: + return None, ( + jsonify({'error': f'Path "{json_path}" is a primitive value; pick an object or array'}), + 400, + ) + + if not rows: + return None, (jsonify({'error': 'Could not extract tabular data from JSON'}), 400) + + return rows, None @bp.route('/process', methods=['POST']) @@ -40,165 +398,114 @@ def health(): def process_json(): """Process JSON data from file upload, pasted text, or API fetch.""" try: - input_method = request.form.get('input_method') - json_data = None - data_format = request.form.get('data_format', 'json') - if input_method == 'file': - if 'json_file' not in request.files: - return jsonify({'error': 'No file uploaded'}), 400 - file = request.files['json_file'] - if file.filename == '': - return jsonify({'error': 'No file selected'}), 400 - try: - content = file.read().decode('utf-8') - if data_format == 'jsonl': - json_data = parse_jsonl(content) - else: - json_data = json.loads(content) - except UnicodeDecodeError: - return jsonify({'error': 'File must be UTF-8 encoded'}), 400 - except (json.JSONDecodeError, ValueError) as e: - return jsonify({'error': f'Invalid data in file: {str(e)}'}), 400 - - elif input_method == 'paste': - pasted_json = request.form.get('pasted_json', '').strip() - if not pasted_json: - return jsonify({'error': 'No JSON provided'}), 400 - try: - if data_format == 'jsonl': - json_data = parse_jsonl(pasted_json) - else: - json_data = json.loads(pasted_json) - except (json.JSONDecodeError, ValueError) as e: - return jsonify({'error': f'Invalid data: {str(e)}'}), 400 - - elif input_method == 'api': - api_url = request.form.get('api_url', '').strip() - if not api_url: - return jsonify({'error': 'No API URL provided'}), 400 - - is_valid, error_msg = validate_url(api_url) - if not is_valid: - return jsonify({'error': error_msg}), 400 - - auth_method = request.form.get('auth_method', 'none') - headers = {} - auth = None - params = {} - - if auth_method == 'api_key': - header_name = request.form.get('api_key_header', 'X-API-Key') - api_key = request.form.get('api_key', '') - if api_key: - headers[header_name] = api_key - elif auth_method == 'basic': - username = request.form.get('basic_username', '') - password = request.form.get('basic_password', '') - if username: - auth = HTTPBasicAuth(username, password) - elif auth_method == 'bearer': - bearer_token = request.form.get('bearer_token', '') - if bearer_token: - headers['Authorization'] = f'Bearer {bearer_token}' - elif auth_method == 'query_param': - param_name = request.form.get('query_param_name', 'api_key') - param_value = request.form.get('query_param_value', '') - if param_value: - params[param_name] = param_value - - try: - timeout = current_app.config['API_FETCH_TIMEOUT'] - max_size = current_app.config['API_FETCH_MAX_RESPONSE'] - - # Use original URL to preserve TLS/SNI verification. - # SSRF mitigated by: pre-request DNS validation + disabled redirects. - # Residual DNS rebinding risk is minimal (requires attacker-controlled - # DNS with sub-millisecond TTL between our check and requests' connect). - resp = requests.get( - api_url, - headers=headers, - auth=auth, - params=params, - timeout=timeout, - stream=True, - allow_redirects=False - ) - resp.raise_for_status() - - content = bytearray() - for chunk in resp.iter_content(chunk_size=8192): - content.extend(chunk) - if len(content) > max_size: - return jsonify({ - 'error': f'API response exceeds maximum size ' - f'({max_size // (1024 * 1024)}MB)' - }), 400 - - text = bytes(content).decode('utf-8') - if data_format == 'jsonl': - json_data = parse_jsonl(text) - else: - json_data = json.loads(text) - - except requests.exceptions.Timeout: - return jsonify({'error': 'API request timed out'}), 400 - except requests.exceptions.RequestException as e: - logger.warning('API request failed: %s', e) - return jsonify({'error': 'API request failed'}), 400 - except json.JSONDecodeError: - return jsonify({'error': 'API response is not valid JSON'}), 400 - else: - return jsonify({'error': 'Invalid input method'}), 400 - - # Check if user selected a specific JSON path - json_path = request.form.get('json_path', '') - - if json_path: - selected = extract_by_path(json_data, json_path) - if selected is None: - return jsonify({'error': f'Path "{json_path}" not found'}), 400 - if isinstance(selected, list): - table_data = extract_table_data(selected) - elif isinstance(selected, dict): - table_data = [selected] - else: - return jsonify({ - 'error': f'Path "{json_path}" is a primitive value; pick an object or array' - }), 400 - else: - # No path chosen yet — let the client render a tree picker - return jsonify({ - 'needs_selection': True, - 'raw_json': json_data - }) - - if not table_data: - return jsonify({'error': 'Could not extract tabular data from JSON'}), 400 + json_data, error_response = _load_input(data_format) + if error_response: + return error_response + + table_data, error_response = _select_table_data( + json_data, request.form.get('json_path', '') + ) + if error_response: + return error_response columns = get_all_columns(table_data) preview_limit = current_app.config['PREVIEW_ROW_LIMIT'] - preview_data = table_data[:preview_limit] - - max_depth = current_app.config['FLATTEN_MAX_DEPTH'] - csv_data = [flatten_for_csv(row, max_depth=max_depth) for row in table_data] - csv_columns = get_all_columns(csv_data) - - return jsonify({ - 'success': True, - 'columns': columns, - 'preview': preview_data, - 'total_rows': len(table_data), - 'csv_data': csv_data, - 'csv_columns': csv_columns - }) - - except Exception as e: + # A separate projection, not a mutation: csv_data below is built from the + # untouched rows, so exports stay full-fidelity (P2.2/P5). + preview_data = [preview_truncate(row) for row in table_data[:preview_limit]] + + # One pass instead of flatten-then-rescan: names are collected into a set + # while flattening and sorted once at the end, which is byte-identical to + # get_all_columns' sorted output (P8). + csv_data, csv_columns = flatten_rows( + table_data, max_depth=current_app.config['FLATTEN_MAX_DEPTH'] + ) + + # Additive only: no existing key changes name, type or meaning. + # total_cells/max_export_cells let the client grey out the Excel entry + # BEFORE the user clicks (D6); preview_limit drives the badge (P11). + return jsonify( + { + 'success': True, + 'columns': columns, + 'preview': preview_data, + 'preview_limit': preview_limit, + 'total_rows': len(table_data), + 'total_cells': len(csv_data) * len(csv_columns), + 'max_export_cells': current_app.config.get( + 'MAX_EXPORT_CELLS', DEFAULT_MAX_EXPORT_CELLS + ), + 'csv_data': csv_data, + 'csv_columns': csv_columns, + } + ) + + except HTTPException: + # Werkzeug raises these lazily inside the route -- RequestEntityTooLarge + # fires the first time the oversized body is read. They already carry the + # right status, so let Flask's error handlers render them as JSON (F10) + # instead of swallowing them into a 500 below. + raise + except RecursionError: + # Valid JSON can nest deeply enough to exhaust the C stack, in json.loads + # itself, in the helpers, or in the response encoder. That is the caller's + # document, so it is a 400 -- and it is not worth an exception traceback + # in the logs (F8). + return jsonify({'error': 'JSON nesting too deep'}), 400 + except Exception: logger.exception('Unexpected error in process_json') return jsonify({'error': 'An internal error occurred'}), 500 +def _stream_csv(columns, rows): + """ + Yield the CSV a chunk of rows at a time. + + csv.writer needs a text buffer, so one StringIO is reused and truncated every + CSV_STREAM_CHUNK_ROWS rows instead of the whole file being built in memory + before the first byte goes out (P3). + """ + buffer = io.StringIO() + writer = csv.writer(buffer) + + def drain(): + value = buffer.getvalue() + buffer.seek(0) + buffer.truncate(0) + return value + + writer.writerow([sanitize_cell(column) for column in columns]) + yield drain() + + for index, row in enumerate(rows, start=1): + writer.writerow([sanitize_cell(row.get(column, '')) for column in columns]) + if index % CSV_STREAM_CHUNK_ROWS == 0: + yield drain() + + remainder = drain() + if remainder: + yield remainder + + +def _append_xlsx_row(ws, values): + """ + Append one row to a worksheet, writing formula-triggering strings as strings. + + openpyxl serializes a str starting with '=' as a formula cell, so Excel would + evaluate an attacker-supplied value on open (F1). XLSX carries an explicit + type per cell, so the fix is to pin data_type rather than mangle the text the + way the delimited exports have to. + """ + serialized = [serialize_cell_value(value) for value in values] + ws.append(serialized) + row_index = ws.max_row + for column_index, value in enumerate(serialized, start=1): + if is_formula_trigger(value): + ws.cell(row=row_index, column=column_index).data_type = 's' + + @bp.route('/export-csv', methods=['POST']) @limiter.limit(lambda: current_app.config.get('RATE_LIMIT_EXPORT', '60/minute')) def export_csv(): @@ -214,30 +521,32 @@ def export_csv(): if not csv_data: return jsonify({'error': 'No data to export'}), 400 - output = io.StringIO() - writer = csv.DictWriter(output, fieldnames=csv_columns, extrasaction='ignore') - writer.writeheader() - - for row in csv_data: - clean_row = {} - for k, v in row.items(): - if isinstance(v, (dict, list)): - clean_row[k] = json.dumps(v) - else: - clean_row[k] = v - writer.writerow(clean_row) - - output.seek(0) + # _stream_csv calls row.get(), and the generator runs AFTER the response + # headers are sent -- so a non-dict row raises inside the WSGI iterator, + # where the except below can no longer reach it, and the client keeps a + # 200 with a truncated body. Before P3 made this streamed the whole file + # was built inside the try and the same payload returned a JSON 500. + # Validating the shape up front is what restores that contract. + if not all(isinstance(row, dict) for row in csv_data): + return jsonify({'error': 'csv_data must be a list of objects'}), 400 + + # Streamed, and deliberately uncapped: CSV is natively streamable with no + # temp files, so every dataset /process accepts stays exportable by this + # route even when it is too large for a workbook (P3/D6). That, not an + # unbounded XLSX path, is what keeps the export contract as wide as the + # input contract. return Response( - output.getvalue(), + _stream_csv(csv_columns, csv_data), mimetype='text/csv', headers={ 'Content-Disposition': 'attachment; filename=exported_data.csv', - 'Content-Type': 'text/csv; charset=utf-8' - } + 'Content-Type': 'text/csv; charset=utf-8', + }, ) - except Exception as e: + except HTTPException: + raise + except Exception: logger.exception('Unexpected error in export_csv') return jsonify({'error': 'Export failed'}), 500 @@ -247,8 +556,6 @@ def export_csv(): def export_xlsx(): """Export data as Excel file.""" try: - from openpyxl import Workbook - data = request.get_json(silent=True) if not data: return jsonify({'error': 'Invalid or missing JSON body'}), 400 @@ -259,35 +566,52 @@ def export_xlsx(): if not xlsx_data: return jsonify({'error': 'No data to export'}), 400 + limit = current_app.config.get('MAX_EXPORT_CELLS', DEFAULT_MAX_EXPORT_CELLS) + cells = len(xlsx_data) * len(xlsx_columns) + if limit and cells > limit: + # Defence in depth for direct API callers; the UI already greyed the + # Excel entry out using total_cells/max_export_cells from /process. + # A refusal, never a silent truncation -- a partial spreadsheet is + # worse than none. + return jsonify( + { + 'error': ( + f'Dataset is {cells} cells, above the Excel export limit ' + f'of {limit}; export CSV or TSV instead.' + ) + } + ), 400 + wb = Workbook() ws = wb.active ws.title = 'Data' - # Header row - ws.append(xlsx_columns) - - # Data rows + _append_xlsx_row(ws, xlsx_columns) for row in xlsx_data: - row_values = [] - for col in xlsx_columns: - v = row.get(col, '') - if isinstance(v, (dict, list)): - v = json.dumps(v) - row_values.append(v) - ws.append(row_values) - + _append_xlsx_row(ws, [row.get(col, '') for col in xlsx_columns]) + + # Normal-mode Workbook and a plain BytesIO: no OS temp files anywhere. + # openpyxl's write_only mode writes worksheet parts to disk, and + # SpooledTemporaryFile is either pointless (its default max_size=0 never + # rolls over, so it is a BytesIO with extra indirection) or disk-backed + # (a non-zero threshold, or any fileno() call, puts payload bytes on + # disk). The cell budget above is what bounds memory (D6). output = io.BytesIO() wb.save(output) + del wb output.seek(0) - return Response( - output.getvalue(), + # send_file streams the buffer out in chunks; getvalue() would make a + # second full copy of the workbook at peak. + return send_file( + output, mimetype='application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', - headers={ - 'Content-Disposition': 'attachment; filename=exported_data.xlsx', - } + as_attachment=True, + download_name='exported_data.xlsx', ) - except Exception as e: + except HTTPException: + raise + except Exception: logger.exception('Unexpected error in export_xlsx') return jsonify({'error': 'Export failed'}), 500 diff --git a/security.py b/security.py index d635e63..f2bb553 100644 --- a/security.py +++ b/security.py @@ -1,11 +1,201 @@ """Security utilities: SSRF protection and response headers.""" +import concurrent.futures import ipaddress +import os import socket +import threading +from typing import Any from urllib.parse import urlparse +from flask import current_app, has_app_context, request -def validate_url(url): +# Built as a list of directives so appending can never fuse two tokens into one +# malformed directive (F5). Google Fonts is the only third-party origin the page +# uses; scripts stay self-only, so no inline JS is possible anywhere. +CSP_DIRECTIVES = ( + "default-src 'self'", + "script-src 'self'", + "style-src 'self' https://fonts.googleapis.com", + "font-src 'self' https://fonts.gstatic.com", + "img-src 'self' data:", + "connect-src 'self'", + # Shrink the XSS blast radius: no plugins, no hijacking, no framing, + # no cross-origin form exfiltration. + "object-src 'none'", + "base-uri 'self'", + "frame-ancestors 'none'", + "form-action 'self'", + # Inert on a plain-http deployment, mandatory on an https one (F5/F14). + 'upgrade-insecure-requests', +) + +CONTENT_SECURITY_POLICY = '; '.join(CSP_DIRECTIVES) + +# Responses that carry payload data (or ops state) must not be retained by a +# shared cache, a proxy or the browser's bfcache (F11). The index page and the +# static assets are deliberately absent -- they are cacheable (P6). +NO_STORE_ENDPOINTS = frozenset( + { + 'main.process_json', + 'main.export_csv', + 'main.export_xlsx', + 'main.health', + 'main.health_live', + 'main.health_ready', + } +) + + +# --- Bounded DNS admission (F6.1 / P7) -------------------------------------- +# +# socket.getaddrinfo takes no timeout and cannot be cancelled, so a hostname +# served by a slow or unresponsive nameserver pins the gunicorn worker that +# called it for as long as the platform resolver takes. The lookup therefore runs +# on a shared, fixed-size pool and the caller waits with a timeout. +# +# What this bounds and what it does not: +# +# * Bounded: how long a REQUEST waits (Future.result timeout), and how many +# lookups may be in flight at once (the admission permits). Concurrency is +# the actual worker-starvation fix. +# * NOT bounded: the lookup itself, and therefore worker teardown. cancel_futures +# only drops queued work, a running getaddrinfo cannot be cancelled, and +# concurrent.futures joins its non-daemon threads at interpreter exit whatever +# `wait` says. The wait is whatever the platform resolver takes -- glibc +# defaults to ~5s per nameserver x 2 attempts x every nameserver in +# resolv.conf, so tens of seconds is the realistic worst case, and it is +# bounded at all only where `options timeout:N attempts:M` is configured. +# Pinning `options timeout:2 attempts:1` in the container's resolv.conf is a +# best-effort narrowing, not a guarantee. A killable subprocess resolver is +# the only real bound and is deliberately out of v1.2 scope. +# +# Do not describe teardown as bounded anywhere. + +DEFAULT_ALLOWED_PORTS = frozenset({80, 443, 8443}) +DEFAULT_SCHEME_PORTS = {'http': 80, 'https': 443} + +DEFAULT_DNS_TIMEOUT = 3 +DEFAULT_DNS_MAX_WORKERS = 4 +DEFAULT_DNS_ADMISSION_TIMEOUT = 1 + + +class ResolverBusyError(Exception): + """No admission permit was free within the admission wait.""" + + +class _ResolverPool: + """A fixed-size resolver pool with an admission permit per in-flight lookup.""" + + def __init__(self, max_workers: int) -> None: + self.pid = os.getpid() + self.max_workers = max_workers + # Pool size plus an equal backlog: a bounded submission queue. Beyond this + # callers are rejected rather than queued without limit. + self.capacity = max_workers * 2 + self.executor = concurrent.futures.ThreadPoolExecutor( + max_workers=max_workers, thread_name_prefix='dns-resolver' + ) + self._permits = threading.Semaphore(self.capacity) + self._counter_lock = threading.Lock() + self.in_flight = 0 + + def submit(self, hostname: str, admission_timeout: float) -> concurrent.futures.Future: + """ + Admit and start one lookup, or raise ResolverBusyError. + + The permit is taken BEFORE submit and released from the future's + done-callback -- never from the caller's finally. Releasing on caller + timeout would re-admit work while the blocked getaddrinfo thread still + occupies the pool, which is precisely how the pool saturates under + repeated slow-DNS requests. + """ + if not self._permits.acquire(timeout=admission_timeout): + raise ResolverBusyError + with self._counter_lock: + self.in_flight += 1 + try: + future = self.executor.submit(socket.getaddrinfo, hostname, None) + except BaseException: + self._release() + raise + future.add_done_callback(self._on_done) + return future + + def _on_done(self, _future: concurrent.futures.Future) -> None: + self._release() + + def _release(self) -> None: + with self._counter_lock: + self.in_flight -= 1 + self._permits.release() + + +_pool_lock = threading.Lock() +_pool = None + + +def get_resolver_pool(max_workers: int | None = None) -> '_ResolverPool': + """ + Return this process's resolver pool, creating it on first use. + + Creation is lazy so the pool belongs to the gunicorn WORKER, not the master: + an executor built at import time in the master leaves its threads behind in + the parent and is not usefully inherited. The recorded pid also makes a pool + inherited across a fork be replaced rather than reused. + """ + global _pool + if max_workers is None: + max_workers = _setting('API_DNS_MAX_WORKERS', DEFAULT_DNS_MAX_WORKERS) + pool = _pool + if pool is not None and pool.pid == os.getpid(): + return pool + with _pool_lock: + if _pool is None or _pool.pid != os.getpid(): + _pool = _ResolverPool(max_workers) + return _pool + + +def reset_resolver_pool() -> '_ResolverPool | None': + """ + Drop the current pool so the next lookup builds a fresh one. + + shutdown(wait=False, cancel_futures=True) returns immediately but does NOT + make teardown bounded: it can only drop queued work, and any thread already + inside getaddrinfo keeps running until the platform resolver returns. + """ + global _pool + with _pool_lock: + pool = _pool + _pool = None + if pool is not None: + pool.executor.shutdown(wait=False, cancel_futures=True) + return pool + + +def _setting(name: str, default: Any) -> Any: + """Read a config value, falling back to the module default outside a request.""" + if has_app_context(): + return current_app.config.get(name, default) + return default + + +def resolve_hostname(hostname: str) -> list[Any]: + """ + Resolve a hostname under admission control. + + Returns getaddrinfo's result, or raises ResolverBusyError (no permit), + TimeoutError (the caller's wait elapsed; the lookup itself keeps running) or + socket.gaierror. + """ + pool = get_resolver_pool() + admission_timeout = _setting('API_DNS_ADMISSION_TIMEOUT', DEFAULT_DNS_ADMISSION_TIMEOUT) + timeout = _setting('API_DNS_TIMEOUT', DEFAULT_DNS_TIMEOUT) + future = pool.submit(hostname, admission_timeout) + return future.result(timeout=timeout) + + +def validate_url(url: str) -> tuple[bool, str | None]: """ Validate a URL for SSRF protection. Resolves DNS and rejects non-global IPs. @@ -20,10 +210,30 @@ def validate_url(url): if not hostname: return False, 'Invalid URL: no hostname' + # Port check first: it is free, and a rejected URL should not cost a lookup. + try: + port = parsed.port + except ValueError: + return False, 'Invalid URL: malformed port' + if port is None: + port = DEFAULT_SCHEME_PORTS[parsed.scheme] + allowed_ports = _setting('API_ALLOWED_PORTS', DEFAULT_ALLOWED_PORTS) + if allowed_ports and port not in allowed_ports: + return False, ( + f'Port {port} is not allowed. Allowed ports: ' + + ', '.join(str(p) for p in sorted(allowed_ports)) + ) + try: - addr_infos = socket.getaddrinfo(hostname, None) + addr_infos = resolve_hostname(hostname) + except ResolverBusyError: + return False, 'DNS resolver is busy; please retry' except socket.gaierror: return False, f'Could not resolve hostname: {hostname}' + except TimeoutError: + # The caller's wait elapsed. The lookup is still running on the pool and + # still holds its permit until it finishes -- that is deliberate. + return False, f'Could not resolve hostname: {hostname}' found_valid = False for addr_info in addr_infos: @@ -45,14 +255,22 @@ def validate_url(url): def apply_security_headers(response): """Add security headers to every response.""" response.headers['X-Content-Type-Options'] = 'nosniff' + # Legacy fallback for browsers predating CSP frame-ancestors. response.headers['X-Frame-Options'] = 'DENY' response.headers['Referrer-Policy'] = 'strict-origin-when-cross-origin' - response.headers['Content-Security-Policy'] = ( - "default-src 'self'; " - "script-src 'self'; " - "style-src 'self' https://fonts.googleapis.com; " - "font-src 'self' https://fonts.gstatic.com; " - "img-src 'self' data:; " - "connect-src 'self'" - ) + response.headers['Permissions-Policy'] = 'camera=(), microphone=(), geolocation=()' + response.headers['Cross-Origin-Opener-Policy'] = 'same-origin' + response.headers['Cross-Origin-Resource-Policy'] = 'same-origin' + response.headers['Content-Security-Policy'] = CONTENT_SECURITY_POLICY + + if request.endpoint in NO_STORE_ENDPOINTS: + response.headers['Cache-Control'] = 'no-store' + + # HSTS only makes sense once the connection is already TLS; sending it over + # plain http would pin a local dev server to https. request.is_secure reads + # X-Forwarded-Proto only when ProxyFix is enabled (TRUST_PROXY=1), which is + # exactly the deployment where the proxy terminates TLS. + if request.is_secure: + response.headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains' + return response diff --git a/static/css/style.css b/static/css/style.css index 1dd04e8..2616f26 100644 --- a/static/css/style.css +++ b/static/css/style.css @@ -954,3 +954,114 @@ th.sort-desc::after { white-space: nowrap; } + +/* Export entry that is out of range for the format (P3/D6). */ +.export-dropdown-item.disabled, +.export-dropdown-item:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +/* --- v1.2 table toolbar: filter, pagination, column visibility (4.1/4.2/4.4) --- */ +.table-toolbar { + display: flex; + flex-wrap: wrap; + gap: 12px; + justify-content: space-between; + align-items: center; + margin-bottom: 12px; +} + +.table-toolbar-group { + display: flex; + gap: 8px; + align-items: center; +} + +.row-filter { + min-width: 240px; +} + +.filter-count, +.row-warning { + font-size: 0.85rem; + color: var(--text-secondary); +} + +.row-warning { + padding: 8px 12px; + margin-bottom: 12px; + border-radius: 6px; + border: 1px solid var(--border-color); +} + +.btn-small { + padding: 6px 12px; + font-size: 0.85rem; +} + +.column-group { + position: relative; +} + +.column-dropdown { + display: none; + position: absolute; + right: 0; + top: calc(100% + 6px); + z-index: 20; + min-width: 220px; + max-height: 320px; + overflow-y: auto; + padding: 8px; + border-radius: 8px; + border: 1px solid var(--border-color); + background: var(--bg-card); +} + +.column-dropdown.visible { + display: block; +} + +.column-dropdown label { + display: flex; + gap: 8px; + align-items: center; + padding: 4px 6px; + font-size: 0.85rem; + cursor: pointer; +} + +.visually-hidden { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; + border: 0; +} + +.text-muted { + color: var(--text-secondary); +} + +/* "Show N more" control inside the tree picker (keeps nodes past the per-level + cap reachable rather than merely announced). */ +.tree-more-button { + display: block; + width: 100%; + text-align: left; + background: none; + border: none; + padding: 4px 0; + font: inherit; + color: var(--accent-primary); + cursor: pointer; +} + +.tree-more-button:hover { + text-decoration: underline; +} diff --git a/static/js/app.js b/static/js/app.js index 6eb5939..91e93be 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -25,6 +25,9 @@ const exportBtn = document.getElementById('exportBtn'); // Store data for export and sorting let csvData = null; let csvColumns = null; +let totalCells = 0; +let maxExportCells = 0; +let previewLimit = 25; let currentColumns = null; let currentRows = null; let currentTotalRows = 0; @@ -138,8 +141,14 @@ async function submitForm(jsonPath) { csvData = data.csv_data; csvColumns = data.csv_columns; - - renderTable(data.columns, data.preview, data.total_rows); + totalCells = data.total_cells || 0; + maxExportCells = data.max_export_cells || 0; + // P11: the badge used to hardcode 25, so changing PREVIEW_ROW_LIMIT gave + // an operator a wrong badge. + previewLimit = data.preview_limit || previewLimit; + updateExcelAvailability(); + + setTableData(data); showResults(); } catch (err) { @@ -158,13 +167,27 @@ const treeCancelBtn = document.getElementById('treeCancel'); let selectedTreePath = null; +// P4: the picker used to build a DOM node for every key of every object up +// front, so a 10 MB payload meant tens of thousands of nodes in one synchronous +// pass -- a multi-second freeze before the modal appeared. Children are now +// built on first toggle, and both the per-level fan-out and the total node count +// are capped. +const TREE_MAX_CHILDREN = 200; +const TREE_MAX_NODES = 5000; + +// Values are held off-DOM: a node's children cannot be built from markup alone. +const treeNodeValues = new WeakMap(); +let treeNodesBuilt = 0; + function showTreePicker(rawJson) { selectedTreePath = null; + treeNodesBuilt = 0; treeSelectedLabel.textContent = 'No node selected'; treeConfirmBtn.disabled = true; treeContainer.innerHTML = ''; treeContainer.appendChild(buildTreeNode(rawJson, '(root)', 'root', true)); pathModal.classList.add('visible'); + preselectPathFromHash(); } function describeNode(value) { @@ -184,6 +207,8 @@ function describeNode(value) { function buildTreeNode(value, path, keyLabel, openByDefault) { const info = describeNode(value); + treeNodesBuilt += 1; + const node = document.createElement('div'); node.classList.add('tree-node', `tree-${info.kind}`); @@ -203,6 +228,7 @@ function buildTreeNode(value, path, keyLabel, openByDefault) { if (info.selectable) { row.classList.add('tree-selectable'); + row.dataset.path = path; row.addEventListener('click', (e) => { // Don't select when clicking only the toggle chevron if (e.target === toggle && hasChildren) return; @@ -220,35 +246,97 @@ function buildTreeNode(value, path, keyLabel, openByDefault) { if (hasChildren) { const children = document.createElement('div'); - children.classList.add('tree-children'); - if (!openByDefault) children.classList.add('hidden'); + children.classList.add('tree-children', 'hidden'); + treeNodeValues.set(children, { value, path }); + node.appendChild(children); + if (openByDefault) { + populateChildren(children); + children.classList.remove('hidden'); + } + } + return node; +} - if (Array.isArray(value)) { - const max = Math.min(value.length, 50); - for (let i = 0; i < max; i++) { - const childPath = path === '(root)' ? String(i) : `${path}.${i}`; - children.appendChild(buildTreeNode(value[i], childPath, `[${i}]`, false)); - } - if (value.length > max) { - const more = document.createElement('div'); - more.classList.add('tree-more'); - more.textContent = `… and ${value.length - max} more items`; - children.appendChild(more); - } +function childEntries(value, path) { + if (Array.isArray(value)) { + return value.map((item, index) => ({ + value: item, + path: path === '(root)' ? String(index) : `${path}.${index}`, + label: `[${index}]`, + })); + } + return Object.keys(value).map(key => ({ + value: value[key], + path: path === '(root)' ? key : `${path}.${key}`, + label: key, + })); +} + +function appendTreeNotice(container, text) { + const notice = document.createElement('div'); + notice.classList.add('tree-more'); + notice.textContent = text; + container.appendChild(notice); +} + +// The per-level cap keeps the first paint cheap, but it must never make a node +// unreachable: without this control, a key past the 200th could be selected +// neither by clicking nor by a #path= deep link. +function appendTreeMoreButton(container, remaining) { + const button = document.createElement('button'); + button.type = 'button'; + button.classList.add('tree-more', 'tree-more-button'); + button.textContent = `Show ${Math.min(remaining, TREE_MAX_CHILDREN)} more of ${remaining}`; + button.addEventListener('click', (e) => { + e.stopPropagation(); + populateNextBatch(container); + }); + container.appendChild(button); +} + +function clearTreeOverflowControl(container) { + const existing = container.querySelector(':scope > .tree-more'); + if (existing) existing.remove(); +} + +// Append the next batch of children. Returns the number now materialized. +function populateNextBatch(children) { + const held = treeNodeValues.get(children); + if (!held) return 0; + + const entries = childEntries(held.value, held.path); + let built = Number(children.dataset.built || 0); + if (built >= entries.length) return built; + + clearTreeOverflowControl(children); + + const target = Math.min(entries.length, built + TREE_MAX_CHILDREN); + while (built < target && treeNodesBuilt < TREE_MAX_NODES) { + const entry = entries[built]; + children.appendChild(buildTreeNode(entry.value, entry.path, entry.label, false)); + built += 1; + } + children.dataset.built = String(built); + + if (built < entries.length) { + if (treeNodesBuilt >= TREE_MAX_NODES) { + appendTreeNotice(children, 'Tree size limit reached — narrow the selection above.'); } else { - for (const k of Object.keys(value)) { - const childPath = path === '(root)' ? k : `${path}.${k}`; - children.appendChild(buildTreeNode(value[k], childPath, k, false)); - } + appendTreeMoreButton(children, entries.length - built); } - node.appendChild(children); } - return node; + return built; +} + +// First batch only; safe to call every time a node is toggled open. +function populateChildren(children) { + if (children.dataset.built === undefined) populateNextBatch(children); } function toggleNode(node, toggle) { const children = node.querySelector(':scope > .tree-children'); if (!children) return; + populateChildren(children); const isHidden = children.classList.toggle('hidden'); toggle.textContent = isHidden ? '▸' : '▾'; } @@ -261,8 +349,89 @@ function selectNode(row, path) { treeConfirmBtn.disabled = false; } +// --- 4.5 deep-linkable path selection ------------------------------------- +// +// #path=users.0.orders pre-selects that node when the picker opens, and +// confirming a selection writes the hash back, so the link can be shared for a +// conversion someone repeats. + +function readPathFromHash() { + const hash = (window.location.hash || '').replace(/^#/, ''); + if (!hash) return null; + const match = new URLSearchParams(hash).get('path'); + return match ? match.trim() : null; +} + +function writePathToHash(path) { + const params = new URLSearchParams(); + params.set('path', path); + window.location.hash = params.toString(); +} + +// Attribute selectors would need escaping for arbitrary JSON keys; scanning the +// rows avoids the question entirely. +function findTreeRowByPath(path) { + return ( + Array.from(treeContainer.querySelectorAll('.tree-row[data-path]')).find( + row => row.dataset.path === path + ) || null + ); +} + +function expandTreeToPath(path) { + if (path === '(root)') { + const rootRow = findTreeRowByPath('(root)'); + if (rootRow) selectNode(rootRow, '(root)'); + return Boolean(rootRow); + } + + let current = ''; + let target = null; + let container = treeContainer.querySelector(':scope > .tree-node > .tree-children'); + + for (const segment of path.split('.')) { + current = current ? `${current}.${segment}` : segment; + + // The target may sit past the per-level cap, so keep materializing + // batches at this level until it appears or the level is exhausted. + let row = findTreeRowByPath(current); + while (!row && container) { + const before = Number(container.dataset.built || 0); + if (populateNextBatch(container) === before) break; + row = findTreeRowByPath(current); + } + // Still missing means the path genuinely does not exist. This is a hint, + // not a command -- leave the picker open rather than erroring. + if (!row) return false; + + const children = row.parentElement.querySelector(':scope > .tree-children'); + container = children; + if (children) { + populateChildren(children); + children.classList.remove('hidden'); + const toggle = row.querySelector('.tree-toggle'); + if (toggle) toggle.textContent = '▾'; + } + target = row; + } + + if (target) { + selectNode(target, current); + target.scrollIntoView({ block: 'nearest' }); + return true; + } + return false; +} + +function preselectPathFromHash() { + const path = readPathFromHash(); + if (!path) return; + expandTreeToPath(path); +} + treeConfirmBtn.addEventListener('click', () => { if (!selectedTreePath) return; + writePathToHash(selectedTreePath); pathModal.classList.remove('visible'); submitForm(selectedTreePath); }); @@ -278,15 +447,119 @@ pathModal.addEventListener('click', (e) => { } }); +// --- Table view model (4.1/4.2/4.4) --------------------------------------- +// +// Two datasets arrive from /process: `preview` (nested, capped server-side) with +// its own `columns`, and `csv_data` (flattened, full fidelity) with +// `csv_columns`. The table starts on the preview. "Load more" switches to the +// flattened dataset -- that is the only one the browser holds for every row -- +// and the badge says so, because the column set genuinely differs (a nested +// `meta` object becomes `meta.age`). + +// Above this many DOM rows the browser starts to struggle; "Load all" asks first. +const MAX_DOM_ROWS = 50000; +const LOAD_MORE_STEP = 500; + +let previewRows = null; +let previewColumns = null; +let viewMode = 'preview'; +let loadedRowCount = 0; +let hiddenColumns = new Set(); +let filterText = ''; + +const rowFilterInput = document.getElementById('rowFilter'); +const filterCount = document.getElementById('filterCount'); +const loadMoreBtn = document.getElementById('loadMoreBtn'); +const loadAllBtn = document.getElementById('loadAllBtn'); +const rowWarning = document.getElementById('rowWarning'); +const columnsBtn = document.getElementById('columnsBtn'); +const columnsDropdown = document.getElementById('columnsDropdown'); + +function setTableData(data) { + previewColumns = data.columns || []; + previewRows = data.preview || []; + currentTotalRows = data.total_rows || 0; + viewMode = 'preview'; + loadedRowCount = previewRows.length; + hiddenColumns = new Set(); + renderedColumnSignature = null; + filterText = ''; + sortColumn = null; + sortDirection = 'asc'; + if (rowFilterInput) rowFilterInput.value = ''; + // Otherwise the "rendering stops at N rows" banner from a previous, larger + // dataset stays up and describes data that is no longer on screen. + showRowWarning(''); + renderTable(); +} + +function baseColumns() { + return (viewMode === 'preview' ? previewColumns : csvColumns) || []; +} + +function baseRows() { + if (viewMode === 'preview') return previewRows || []; + return (csvData || []).slice(0, loadedRowCount); +} + +function visibleColumns() { + return baseColumns().filter(col => !hiddenColumns.has(col)); +} + +function rowMatchesFilter(row, needle) { + return Object.values(row).some(value => { + if (value === null || value === undefined) return false; + const text = typeof value === 'object' ? JSON.stringify(value) : String(value); + return text.toLowerCase().includes(needle); + }); +} + +function applyFilter(rows) { + const needle = filterText.trim().toLowerCase(); + if (!needle) return rows; + return rows.filter(row => rowMatchesFilter(row, needle)); +} + +function compareValues(a, b, col) { + let valA = a[col]; + let valB = b[col]; + + if (valA === null || valA === undefined) valA = ''; + if (valB === null || valB === undefined) valB = ''; + + if (typeof valA === 'object') valA = JSON.stringify(valA); + if (typeof valB === 'object') valB = JSON.stringify(valB); + + if (typeof valA === 'number' && typeof valB === 'number') { + return sortDirection === 'asc' ? valA - valB : valB - valA; + } + + const strA = String(valA).toLowerCase(); + const strB = String(valB).toLowerCase(); + if (strA < strB) return sortDirection === 'asc' ? -1 : 1; + if (strA > strB) return sortDirection === 'asc' ? 1 : -1; + return 0; +} + +function applySort(rows) { + if (!sortColumn) return rows; + return [...rows].sort((a, b) => compareValues(a, b, sortColumn)); +} + // Render table -function renderTable(columns, rows, totalRows) { - currentColumns = columns; - currentRows = rows; - currentTotalRows = totalRows; - renderTableDOM(columns, rows, totalRows); +function renderTable() { + const columns = visibleColumns(); + const loaded = baseRows(); + const filtered = applyFilter(loaded); + const rows = applySort(filtered); + + renderTableDOM(columns, rows); + renderColumnToggles(); + updateCounts(loaded.length, filtered.length); + updateLoadControls(); } -function renderTableDOM(columns, rows, totalRows) { +function renderTableDOM(columns, rows) { tableHead.innerHTML = ''; tableBody.innerHTML = ''; @@ -304,60 +577,204 @@ function renderTableDOM(columns, rows, totalRows) { }); tableHead.appendChild(headerRow); - // Body + // Body. One fragment, so a large "load all" is a single reflow. + const fragment = document.createDocumentFragment(); rows.forEach(row => { const tr = document.createElement('tr'); columns.forEach(col => { const td = document.createElement('td'); - const value = row[col]; - td.innerHTML = formatValue(value); + td.innerHTML = formatValue(row[col]); tr.appendChild(td); }); - tableBody.appendChild(tr); + fragment.appendChild(tr); }); + tableBody.appendChild(fragment); +} + +function updateCounts(loadedCount, shownCount) { + rowCountText.textContent = `${currentTotalRows} total rows`; + + if (filterCount) { + filterCount.textContent = filterText.trim() + ? `${shownCount} of ${loadedCount} loaded rows match` + : ''; + } - // Update counts - rowCountText.textContent = `${totalRows} total rows`; - if (totalRows > 25) { - previewBadge.textContent = 'Showing first 25'; + if (loadedCount < currentTotalRows) { + previewBadge.textContent = `Showing first ${loadedCount}`; + // Sorting and filtering act on the rows currently loaded, while exports + // always contain every row -- say so rather than leaving the discrepancy + // invisible (P11). + previewBadge.title = + `Preview limit is ${previewLimit}. Sorting and filtering apply to the ` + + 'rows loaded here; exports always contain all rows.'; previewBadge.classList.remove('hidden'); } else { - previewBadge.textContent = `Showing all ${totalRows}`; - previewBadge.classList.add('hidden'); + previewBadge.textContent = + viewMode === 'full' + ? `Showing all ${currentTotalRows} (flattened columns)` + : `Showing all ${currentTotalRows}`; + previewBadge.title = + viewMode === 'full' + ? 'Rows past the preview come from the flattened dataset, so nested ' + + 'objects appear as dotted columns.' + : ''; + if (viewMode === 'full') { + previewBadge.classList.remove('hidden'); + } else { + previewBadge.classList.add('hidden'); + } } } -// Column sorting (client-side on preview rows) -function handleSort(col) { - if (sortColumn === col) { - sortDirection = sortDirection === 'asc' ? 'desc' : 'asc'; +function updateLoadControls() { + const total = currentTotalRows; + // Past MAX_DOM_ROWS loadRows() clamps, so further clicks would re-render the + // same rows and the buttons would look broken. + const moreAvailable = loadedRowCount < total && loadedRowCount < MAX_DOM_ROWS; + if (loadMoreBtn) { + loadMoreBtn.disabled = !moreAvailable; + loadMoreBtn.textContent = moreAvailable + ? `Load next ${Math.min(LOAD_MORE_STEP, total - loadedRowCount)}` + : (loadedRowCount >= MAX_DOM_ROWS ? 'Render limit reached' : 'All rows loaded'); + } + if (loadAllBtn) loadAllBtn.disabled = !moreAvailable; +} + +function showRowWarning(message) { + if (!rowWarning) return; + if (!message) { + rowWarning.classList.add('hidden'); + rowWarning.textContent = ''; + return; + } + rowWarning.textContent = message; + rowWarning.classList.remove('hidden'); +} + +function loadRows(count) { + if (!csvData) return; + // Rows past the preview only exist in the flattened dataset. + viewMode = 'full'; + const target = Math.min(loadedRowCount + count, csvData.length); + + if (target > MAX_DOM_ROWS) { + loadedRowCount = Math.min(target, MAX_DOM_ROWS); + showRowWarning( + `Rendering stops at ${MAX_DOM_ROWS} rows to keep the page responsive. ` + + `All ${currentTotalRows} rows are still included in every export.` + ); } else { - sortColumn = col; - sortDirection = 'asc'; + loadedRowCount = target; + showRowWarning(''); } + renderTable(); +} - const sorted = [...currentRows].sort((a, b) => { - let valA = a[col]; - let valB = b[col]; +if (loadMoreBtn) loadMoreBtn.addEventListener('click', () => loadRows(LOAD_MORE_STEP)); +if (loadAllBtn) { + loadAllBtn.addEventListener('click', () => loadRows(Number.MAX_SAFE_INTEGER)); +} - if (valA === null || valA === undefined) valA = ''; - if (valB === null || valB === undefined) valB = ''; +const FILTER_DEBOUNCE_MS = 150; +let filterDebounce = null; + +if (rowFilterInput) { + rowFilterInput.addEventListener('input', () => { + // renderTable() re-filters, re-sorts and rebuilds every cell, so running + // it per keystroke blocks the main thread on a fully loaded dataset. + clearTimeout(filterDebounce); + filterDebounce = setTimeout(() => { + filterText = rowFilterInput.value; + renderTable(); + }, FILTER_DEBOUNCE_MS); + }); +} - if (typeof valA === 'object') valA = JSON.stringify(valA); - if (typeof valB === 'object') valB = JSON.stringify(valB); +// --- Column visibility (4.4) ---------------------------------------------- +let renderedColumnSignature = null; - if (typeof valA === 'number' && typeof valB === 'number') { - return sortDirection === 'asc' ? valA - valB : valB - valA; - } +function renderColumnToggles() { + if (!columnsDropdown) return; - const strA = String(valA).toLowerCase(); - const strB = String(valB).toLowerCase(); - if (strA < strB) return sortDirection === 'asc' ? -1 : 1; - if (strA > strB) return sortDirection === 'asc' ? 1 : -1; - return 0; + const columns = baseColumns(); + const signature = JSON.stringify(columns); + if (signature === renderedColumnSignature) { + // Same columns: only the checked state can have changed. Rebuilding here + // would destroy the checkbox a keyboard user just activated and drop + // focus to document.body. + columnsDropdown.querySelectorAll('input[type=checkbox]').forEach(box => { + box.checked = !hiddenColumns.has(box.dataset.column); + }); + return; + } + renderedColumnSignature = signature; + + columnsDropdown.innerHTML = ''; + columns.forEach(col => { + const label = document.createElement('label'); + const checkbox = document.createElement('input'); + checkbox.type = 'checkbox'; + checkbox.dataset.column = col; + checkbox.checked = !hiddenColumns.has(col); + checkbox.addEventListener('change', () => { + if (checkbox.checked) { + hiddenColumns.delete(col); + } else { + hiddenColumns.add(col); + } + renderTable(); + }); + const text = document.createElement('span'); + text.textContent = col; + label.appendChild(checkbox); + label.appendChild(text); + columnsDropdown.appendChild(label); }); +} - renderTableDOM(currentColumns, sorted, currentTotalRows); +if (columnsBtn) { + columnsBtn.addEventListener('click', (e) => { + e.stopPropagation(); + const open = columnsDropdown.classList.toggle('visible'); + columnsBtn.setAttribute('aria-expanded', String(open)); + }); +} + +document.addEventListener('click', (e) => { + if (columnsDropdown && !e.target.closest('.column-group')) { + columnsDropdown.classList.remove('visible'); + if (columnsBtn) columnsBtn.setAttribute('aria-expanded', 'false'); + } +}); + +// Column sorting (client-side, over the rows currently loaded) +function handleSort(col) { + if (sortColumn === col) { + sortDirection = sortDirection === 'asc' ? 'desc' : 'asc'; + } else { + sortColumn = col; + sortDirection = 'asc'; + } + renderTable(); +} + +// P5: a nested object with 50k keys, a 100k-item array stringified whole, or a +// single 5 MB string cell each freeze the tab. The server caps the preview +// projection too (2.4); these caps also protect rows loaded client-side (4.1). +const RENDER_MAX_KEYS = 20; +const RENDER_MAX_ARRAY_ITEMS = 20; +const RENDER_MAX_STRING = 500; + +function truncateForRender(text, max) { + const str = String(text); + if (str.length <= max) return { text: str, truncated: false }; + return { text: str.slice(0, max), truncated: true }; +} + +function renderTruncatable(value, max) { + const { text, truncated } = truncateForRender(value, max); + return escapeHtml(text) + (truncated ? ' … (truncated)' : ''); } // Format cell value @@ -369,10 +786,17 @@ function formatValue(value) { if (typeof value === 'object') { if (Array.isArray(value)) { if (value.length === 0) return '[]'; - if (typeof value[0] === 'object') { + if (typeof value[0] === 'object' && value[0] !== null) { return renderNestedTable(value); } - return escapeHtml(JSON.stringify(value)); + // Stringify only the head of a primitive array: JSON.stringify over a + // 100k-item array produces one huge string in a single . + const head = value.slice(0, RENDER_MAX_ARRAY_ITEMS); + const rendered = escapeHtml(JSON.stringify(head)); + if (value.length > RENDER_MAX_ARRAY_ITEMS) { + return `${rendered} … and ${value.length - RENDER_MAX_ARRAY_ITEMS} more`; + } + return rendered; } return renderNestedObject(value); } @@ -383,7 +807,7 @@ function formatValue(value) { : 'false'; } - return escapeHtml(String(value)); + return renderTruncatable(value, RENDER_MAX_STRING); } // Render nested object as mini table @@ -391,15 +815,19 @@ function renderNestedObject(obj) { const keys = Object.keys(obj); if (keys.length === 0) return '{}'; + const shown = keys.slice(0, RENDER_MAX_KEYS); let html = ''; - keys.forEach(key => { + shown.forEach(key => { const val = obj[key]; let displayVal = val; if (typeof val === 'object' && val !== null) { displayVal = JSON.stringify(val); } - html += ``; + html += ``; }); + if (keys.length > shown.length) { + html += ``; + } html += '
${escapeHtml(key)}${escapeHtml(String(displayVal))}
${escapeHtml(key)}${renderTruncatable(displayVal, RENDER_MAX_STRING)}
… and ${keys.length - shown.length} more keys
'; return html; } @@ -408,29 +836,37 @@ function renderNestedObject(obj) { function renderNestedTable(arr) { if (arr.length === 0) return '[]'; - const cols = [...new Set(arr.flatMap(item => typeof item === 'object' ? Object.keys(item) : []))]; - if (cols.length === 0) return escapeHtml(JSON.stringify(arr)); + const allCols = [...new Set(arr.flatMap(item => (typeof item === 'object' && item !== null) ? Object.keys(item) : []))]; + if (allCols.length === 0) return escapeHtml(JSON.stringify(arr.slice(0, RENDER_MAX_ARRAY_ITEMS))); + const cols = allCols.slice(0, RENDER_MAX_KEYS); let html = ''; cols.forEach(col => { html += ``; }); + if (allCols.length > cols.length) { + html += ``; + } html += ''; arr.slice(0, 5).forEach(item => { html += ''; cols.forEach(col => { - let val = item[col]; + let val = item ? item[col] : undefined; if (typeof val === 'object' && val !== null) { val = JSON.stringify(val); } - html += ``; + html += ``; }); + if (allCols.length > cols.length) { + html += ''; + } html += ''; }); if (arr.length > 5) { - html += ``; + const span = cols.length + (allCols.length > cols.length ? 1 : 0); + html += ``; } html += '
${escapeHtml(col)}… +${allCols.length - cols.length}
${escapeHtml(String(val ?? ''))}${renderTruncatable(val ?? '', RENDER_MAX_STRING)}
... and ${arr.length - 5} more rows
... and ${arr.length - 5} more rows
'; @@ -444,23 +880,97 @@ function escapeHtml(text) { return div.innerHTML; } +// P3/D6: tell the user Excel is out of range before they click, rather than +// after a 400. CSV/TSV are streamed and uncapped, so there is always a way out. +function isExcelBlocked(cells, limit) { + return limit > 0 && cells > limit; +} + +function excelExportBlocked() { + return isExcelBlocked(totalCells, maxExportCells); +} + +function updateExcelAvailability() { + const item = document.querySelector('.export-dropdown-item[data-format="xlsx"]'); + if (!item) return; + if (excelExportBlocked()) { + item.disabled = true; + item.classList.add('disabled'); + item.textContent = 'Excel — too large, use CSV/TSV'; + item.title = `${totalCells} cells exceeds the Excel export limit of ${maxExportCells}`; + } else { + item.disabled = false; + item.classList.remove('disabled'); + item.textContent = 'Export Excel'; + item.title = ''; + } +} + // Export dropdown toggle const exportDropdown = document.getElementById('exportDropdown'); + +function setExportDropdownOpen(open) { + exportDropdown.classList.toggle('visible', open); + exportBtn.setAttribute('aria-expanded', String(open)); +} + exportBtn.addEventListener('click', () => { - exportDropdown.classList.toggle('visible'); + setExportDropdownOpen(!exportDropdown.classList.contains('visible')); }); // Close dropdown when clicking outside document.addEventListener('click', (e) => { if (!e.target.closest('.export-group')) { - exportDropdown.classList.remove('visible'); + setExportDropdownOpen(false); + } +}); + +// Keyboard: Escape closes and returns focus to the trigger; arrows walk the menu. +document.addEventListener('keydown', (e) => { + if (e.key !== 'Escape') return; + if (exportDropdown.classList.contains('visible')) { + setExportDropdownOpen(false); + exportBtn.focus(); + } + if (columnsDropdown && columnsDropdown.classList.contains('visible')) { + columnsDropdown.classList.remove('visible'); + if (columnsBtn) { + columnsBtn.setAttribute('aria-expanded', 'false'); + columnsBtn.focus(); + } + } + if (aboutModal && aboutModal.classList.contains('visible')) { + closeAboutModal(); + } + if (pathModal.classList.contains('visible')) { + pathModal.classList.remove('visible'); + } +}); + +exportDropdown.addEventListener('keydown', (e) => { + const items = Array.from(exportDropdown.querySelectorAll('.export-dropdown-item')); + const index = items.indexOf(document.activeElement); + if (e.key === 'ArrowDown') { + e.preventDefault(); + items[(index + 1) % items.length].focus(); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + items[(index - 1 + items.length) % items.length].focus(); } }); +exportBtn.addEventListener('keydown', (e) => { + if (e.key !== 'ArrowDown') return; + e.preventDefault(); + setExportDropdownOpen(true); + const first = exportDropdown.querySelector('.export-dropdown-item'); + if (first) first.focus(); +}); + // Export handlers document.querySelectorAll('.export-dropdown-item').forEach(item => { item.addEventListener('click', async () => { - exportDropdown.classList.remove('visible'); + setExportDropdownOpen(false); const format = item.dataset.format; if (!csvData || !csvColumns) { @@ -472,14 +982,63 @@ document.querySelectorAll('.export-dropdown-item').forEach(item => { downloadDelimited(csvColumns, csvData, ',', 'exported_data.csv'); } else if (format === 'tsv') { downloadDelimited(csvColumns, csvData, '\t', 'exported_data.tsv'); + } else if (format === 'jsonl') { + downloadChunks( + buildJsonlChunks(csvColumns, csvData), + 'application/x-ndjson; charset=utf-8', + 'exported_data.jsonl' + ); + } else if (format === 'markdown') { + downloadChunks( + buildMarkdownChunks(csvColumns, csvData), + 'text/markdown; charset=utf-8', + 'exported_data.md' + ); } else if (format === 'xlsx') { + if (excelExportBlocked()) { + showError( + `Dataset is ${totalCells} cells, above the Excel export limit of ` + + `${maxExportCells}. Export CSV or TSV instead.` + ); + return; + } await exportXlsx(); } }); }); -// Client-side CSV/TSV generation -function downloadDelimited(columns, data, delimiter, filename) { +// Spreadsheet formula triggers (OWASP). Must stay in sync with +// helpers.FORMULA_TRIGGERS on the server. +const FORMULA_TRIGGERS = ['=', '+', '-', '@', '\t', '\r', '\n']; + +// Reduce a cell to the scalar a writer emits. Containers become their JSON text. +function serializeCellValue(value) { + if (value === null || value === undefined) return ''; + if (typeof value === 'object') return JSON.stringify(value); + return value; +} + +// Defuse CSV/TSV formula injection (CWE-1236) by prefixing a dangerous value +// with a single quote. Delimited output carries no type channel, so this is the +// only place the value can be marked as text. JSONL and Markdown exports are +// deliberately NOT routed through here. +function sanitizeCell(value) { + const serialized = serializeCellValue(value); + if (typeof serialized === 'string' && FORMULA_TRIGGERS.some(t => serialized.startsWith(t))) { + return "'" + serialized; + } + return serialized; +} + +// P13: build the file as a list of chunks instead of one giant string. A 10 MB +// dataset otherwise means a ~10-30 MB string plus a Blob copy of it, which +// blocks the main thread and doubles peak memory for no reason -- Blob already +// accepts several parts. +const BLOB_CHUNK_ROWS = 2000; + +// Pure builders, kept separate from the download so they can be asserted +// directly (tests/js/test_export_sanitize.mjs). +function buildDelimitedChunks(columns, data, delimiter) { const escape = (val) => { const str = String(val ?? ''); if (str.includes(delimiter) || str.includes('"') || str.includes('\n')) { @@ -488,20 +1047,94 @@ function downloadDelimited(columns, data, delimiter, filename) { return str; }; - let output = columns.map(escape).join(delimiter) + '\n'; - data.forEach(row => { - const line = columns.map(col => { - let v = row[col]; - if (typeof v === 'object' && v !== null) v = JSON.stringify(v); - return escape(v); - }).join(delimiter); - output += line + '\n'; + const chunks = []; + let pending = columns.map(col => escape(sanitizeCell(col))).join(delimiter) + '\n'; + + data.forEach((row, index) => { + pending += columns.map(col => escape(sanitizeCell(row[col]))).join(delimiter) + '\n'; + if ((index + 1) % BLOB_CHUNK_ROWS === 0) { + chunks.push(pending); + pending = ''; + } }); - const mimeType = delimiter === '\t' - ? 'text/tab-separated-values; charset=utf-8' - : 'text/csv; charset=utf-8'; - const blob = new Blob([output], { type: mimeType }); + if (pending) chunks.push(pending); + return chunks; +} + +function buildDelimited(columns, data, delimiter) { + return buildDelimitedChunks(columns, data, delimiter).join(''); +} + +// --- 4.3 JSONL export ----------------------------------------------------- +// +// Values go out UNESCAPED -- no formula sanitization. F1's sanitizer exists +// because CSV/TSV/XLSX have no type channel and a spreadsheet re-interprets a +// leading '=' as a formula; JSON has types, nothing evaluates it, and prefixing +// values here would corrupt the data instead of protecting anything. +// +// NOT a faithful copy of the input document: rows come from csv_data, which the +// server already flattened, so nested objects appear as dotted keys and nested +// arrays as JSON strings. The unflattened rows are never sent to the browser -- +// `preview` is both truncated and capped at preview_limit rows -- so a +// round-tripping JSONL export would require shipping the original rows too, +// which is exactly the payload/memory cost P2 and P12 set out to avoid. +function buildJsonlChunks(columns, data) { + const chunks = []; + let pending = ''; + data.forEach((row, index) => { + const projected = {}; + columns.forEach(col => { + if (row[col] !== undefined) projected[col] = row[col]; + }); + pending += JSON.stringify(projected) + '\n'; + if ((index + 1) % BLOB_CHUNK_ROWS === 0) { + chunks.push(pending); + pending = ''; + } + }); + if (pending) chunks.push(pending); + return chunks; +} + +// --- 4.3 Markdown export -------------------------------------------------- +// +// Markdown-specific escaping only, again NOT the spreadsheet sanitizer: a +// leading '=' is inert in Markdown. What does break a Markdown table is an +// unescaped pipe or a newline inside a cell. +function escapeMarkdownCell(value) { + if (value === null || value === undefined) return ''; + const text = typeof value === 'object' ? JSON.stringify(value) : String(value); + return text + .replace(/\\/g, '\\\\') + // Markdown permits raw HTML, so an unescaped '<' carries a live tag -- + // `` -- into the .md file and executes wherever + // it is rendered. Escaped before the newline rule below so the '<' of + // the
that rule injects is not itself escaped. + .replace(/'); +} + +function buildMarkdownChunks(columns, data) { + const chunks = []; + let pending = + '| ' + columns.map(escapeMarkdownCell).join(' | ') + ' |\n' + + '| ' + columns.map(() => '---').join(' | ') + ' |\n'; + + data.forEach((row, index) => { + pending += '| ' + columns.map(col => escapeMarkdownCell(row[col])).join(' | ') + ' |\n'; + if ((index + 1) % BLOB_CHUNK_ROWS === 0) { + chunks.push(pending); + pending = ''; + } + }); + if (pending) chunks.push(pending); + return chunks; +} + +function downloadChunks(chunks, mimeType, filename) { + const blob = new Blob(chunks, { type: mimeType }); const url = window.URL.createObjectURL(blob); const a = document.createElement('a'); a.href = url; @@ -512,6 +1145,16 @@ function downloadDelimited(columns, data, delimiter, filename) { a.remove(); } +// Client-side CSV/TSV generation. The blob/anchor dance lives in +// downloadChunks alone -- duplicating it here meant a fix to one copy (an +// unrevoked object URL, say) silently missed CSV and TSV. +function downloadDelimited(columns, data, delimiter, filename) { + const mimeType = delimiter === '\t' + ? 'text/tab-separated-values; charset=utf-8' + : 'text/csv; charset=utf-8'; + downloadChunks(buildDelimitedChunks(columns, data, delimiter), mimeType, filename); +} + // Server-side Excel export (needs openpyxl) async function exportXlsx() { try { @@ -527,7 +1170,10 @@ async function exportXlsx() { }) }); - if (!response.ok) throw new Error('Excel export failed'); + if (!response.ok) { + const detail = await response.json().catch(() => null); + throw new Error((detail && detail.error) || 'Excel export failed'); + } const blob = await response.blob(); const url = window.URL.createObjectURL(blob); @@ -573,10 +1219,39 @@ themeToggle.addEventListener('click', () => { applyTheme(next); }); -// About link -document.getElementById('aboutLink').addEventListener('click', (e) => { +// About dialog. Replaces alert(), which also hardcoded a version string that +// went stale the moment APP_VERSION changed -- the modal reads it from config. +const aboutModal = document.getElementById('aboutModal'); +const aboutLink = document.getElementById('aboutLink'); +const aboutCloseBtn = document.getElementById('aboutClose'); + +// Opening left focus on the trigger, so the dialog content sat outside the tab +// order and closing stranded focus at the top of the document. The export and +// columns dropdowns already move focus in and hand it back; these two helpers +// give the modal the same contract across all three close paths (button, +// overlay click, Escape). +function openAboutModal() { + aboutModal.classList.add('visible'); + aboutCloseBtn.focus(); +} + +function closeAboutModal() { + if (!aboutModal.classList.contains('visible')) return; + aboutModal.classList.remove('visible'); + aboutLink.focus(); +} + +aboutLink.addEventListener('click', (e) => { e.preventDefault(); - alert('JSON Table Converter v1.1.0\n\nBuilt with Flask + Python\nNo data is ever stored or logged.'); + openAboutModal(); +}); + +aboutCloseBtn.addEventListener('click', closeAboutModal); + +aboutModal.addEventListener('click', (e) => { + if (e.target === e.currentTarget) { + closeAboutModal(); + } }); // UI helpers diff --git a/templates/index.html b/templates/index.html index e3fdf24..16ab0d7 100644 --- a/templates/index.html +++ b/templates/index.html @@ -8,7 +8,7 @@ - +
@@ -191,19 +191,40 @@

JSON → Table

- -
- - - +
+
+
+ + + +
+
+ + +
+ +
+
+
+
+
@@ -217,10 +238,24 @@

JSON → Table

+ + + /g) || []).length; + assert.equal(rowCount, 21, 'expected 20 keys plus one "more" row'); + assert.ok(html.includes('49980 more keys')); +}); + +check(() => { + const small = { a: 1, b: 2 }; + const html = renderNestedObject(small); + assert.equal((html.match(//g) || []).length, 2); + assert.ok(!html.includes('more keys')); +}); + +// --- P5: primitive array stringify cap ------------------------------------ +check(() => { + const big = Array.from({ length: 100000 }, (_, i) => i); + const html = formatValue(big); + assert.ok(html.length < 500, `rendered ${html.length} chars for a 100k array`); + assert.ok(html.includes('and 99980 more')); +}); + +check(() => { + assert.equal(formatValue([1, 2, 3]), '[1,2,3]'); + assert.equal(formatValue([]), '[]'); +}); + +// --- P5: long string cap --------------------------------------------------- +check(() => { + const html = formatValue('z'.repeat(5 * 1024 * 1024)); + assert.ok(html.length < 2000, `rendered ${html.length} chars for a 5 MB string`); + assert.ok(html.includes('truncated')); +}); + +check(() => { + assert.equal(formatValue('short'), 'short'); + assert.equal(truncateForRender('abc', 10).truncated, false); + assert.equal(truncateForRender('abcdef', 3).text, 'abc'); +}); + +// --- P5: nested table column cap ------------------------------------------ +check(() => { + const row = {}; + for (let i = 0; i < 200; i += 1) row[`c${i}`] = i; + const html = renderNestedTable([row, row, row, row, row, row, row]); + assert.equal((html.match(/
/g) || []).length, 21, '20 columns plus the overflow header'); + assert.ok(html.includes('... and 2 more rows')); +}); + +// --- P4: tree children are enumerable lazily ------------------------------ +check(() => { + const entries = childEntries({ a: 1, b: 2 }, '(root)'); + assert.deepEqual(Array.from(entries, e => e.path), ['a', 'b']); + assert.deepEqual(Array.from(entries, e => e.label), ['a', 'b']); +}); + +check(() => { + const entries = childEntries([10, 20], 'users'); + assert.deepEqual(Array.from(entries, e => e.path), ['users.0', 'users.1']); + assert.deepEqual(Array.from(entries, e => e.label), ['[0]', '[1]']); +}); + +// --- P13: chunked blob parts ---------------------------------------------- +check(() => { + const rows = Array.from({ length: 5000 }, (_, i) => ({ a: i })); + const chunks = buildDelimitedChunks(['a'], rows, ','); + assert.ok(chunks.length > 1, 'expected the output to be split into parts'); + // Joined chunks are byte-identical to the single-string builder. + assert.equal(chunks.join(''), buildDelimited(['a'], rows, ',')); + const lines = chunks.join('').trim().split('\n'); + assert.equal(lines.length, 5001); + assert.equal(lines[1], '0'); + assert.equal(lines[5000], '4999'); +}); + +check(() => { + // A small dataset still produces exactly one part. + assert.equal(buildDelimitedChunks(['a'], [{ a: 1 }], ',').length, 1); +}); + +// --- D6: the Excel entry is gated on the advertised budget ---------------- +// +// The rule lives in the pure isExcelBlocked(cells, limit) so both directions can +// be asserted: the module-level totalCells/maxExportCells are `let` bindings, +// which vm.runInContext keeps in script scope rather than exposing on the +// context, so a test cannot drive them. +check(() => { + const { isExcelBlocked } = context; + assert.equal(typeof isExcelBlocked, 'function'); + + // Over the limit -> blocked. This is the case the guard exists for. + assert.equal(isExcelBlocked(250001, 250000), true); + assert.equal(isExcelBlocked(1_000_000, 250000), true); + + // At or under the limit -> allowed. + assert.equal(isExcelBlocked(250000, 250000), false); + assert.equal(isExcelBlocked(1, 250000), false); + + // limit 0 disables the guard entirely, however large the dataset. + assert.equal(isExcelBlocked(10_000_000, 0), false); + + // Nothing loaded yet. + assert.equal(isExcelBlocked(0, 250000), false); +}); + +check(() => { + assert.equal(typeof excelExportBlocked, 'function'); + assert.equal(excelExportBlocked(), false, 'no data loaded means nothing to block'); +}); + +console.log(`ok - ${checks} client render/cap assertions passed`); diff --git a/tests/test_helpers.py b/tests/test_helpers.py index bd9e2b3..b5a3fd3 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,84 +1,93 @@ """Tests for data processing helpers.""" import json + from helpers import ( - flatten_for_csv, extract_table_data, get_all_columns, parse_jsonl, - find_candidate_arrays, extract_by_path + PREVIEW_MAX_ITEMS, + PREVIEW_MAX_STRING, + PREVIEW_TRUNCATION_SUFFIX, + extract_by_path, + extract_table_data, + flatten_for_csv, + flatten_rows, + get_all_columns, + parse_jsonl, + preview_truncate, ) class TestFlattenForCsv: def test_flat_dict(self): - result = flatten_for_csv({"a": 1, "b": "hello"}) - assert result == {"a": 1, "b": "hello"} + result = flatten_for_csv({'a': 1, 'b': 'hello'}) + assert result == {'a': 1, 'b': 'hello'} def test_nested_dict(self): - result = flatten_for_csv({"a": {"b": 1, "c": 2}}) - assert result == {"a.b": 1, "a.c": 2} + result = flatten_for_csv({'a': {'b': 1, 'c': 2}}) + assert result == {'a.b': 1, 'a.c': 2} def test_deeply_nested(self): - result = flatten_for_csv({"a": {"b": {"c": {"d": 42}}}}) - assert result == {"a.b.c.d": 42} + result = flatten_for_csv({'a': {'b': {'c': {'d': 42}}}}) + assert result == {'a.b.c.d': 42} def test_list_becomes_json_string(self): - result = flatten_for_csv({"tags": [1, 2, 3]}) - assert result == {"tags": "[1, 2, 3]"} + result = flatten_for_csv({'tags': [1, 2, 3]}) + assert result == {'tags': '[1, 2, 3]'} def test_empty_dict(self): result = flatten_for_csv({}) assert result == {} def test_max_depth_stops_recursion(self): - deep = {"a": {"b": {"c": {"d": "value"}}}} + deep = {'a': {'b': {'c': {'d': 'value'}}}} result = flatten_for_csv(deep, max_depth=2) # At depth 2, the remaining dict should be JSON-serialized - assert "a.b" in result - assert isinstance(result["a.b"], str) - parsed = json.loads(result["a.b"]) - assert parsed == {"c": {"d": "value"}} + assert 'a.b' in result + assert isinstance(result['a.b'], str) + parsed = json.loads(result['a.b']) + assert parsed == {'c': {'d': 'value'}} def test_primitive_value(self): - result = flatten_for_csv("hello", parent_key="key") - assert result == {"key": "hello"} + result = flatten_for_csv('hello', parent_key='key') + assert result == {'key': 'hello'} def test_mixed_types(self): - data = {"name": "Alice", "meta": {"age": 30}, "scores": [90, 85]} + data = {'name': 'Alice', 'meta': {'age': 30}, 'scores': [90, 85]} result = flatten_for_csv(data) - assert result["name"] == "Alice" - assert result["meta.age"] == 30 - assert result["scores"] == "[90, 85]" + assert result['name'] == 'Alice' + assert result['meta.age'] == 30 + assert result['scores'] == '[90, 85]' class TestExtractTableData: def test_array_of_objects(self): - data = [{"id": 1}, {"id": 2}] + data = [{'id': 1}, {'id': 2}] assert extract_table_data(data) == data def test_array_of_primitives(self): result = extract_table_data([1, 2, 3]) - assert result == [{"value": 1}, {"value": 2}, {"value": 3}] + assert result == [{'value': 1}, {'value': 2}, {'value': 3}] def test_dict_with_array_property(self): - data = {"results": [{"id": 1}, {"id": 2}]} - assert extract_table_data(data) == [{"id": 1}, {"id": 2}] + data = {'results': [{'id': 1}, {'id': 2}]} + assert extract_table_data(data) == [{'id': 1}, {'id': 2}] def test_dict_with_primitive_array(self): - data = {"items": ["a", "b"]} - assert extract_table_data(data) == [{"value": "a"}, {"value": "b"}] + data = {'items': ['a', 'b']} + assert extract_table_data(data) == [{'value': 'a'}, {'value': 'b'}] def test_nested_dict_with_array(self): - data = {"data": {"users": [{"name": "Alice"}]}} - assert extract_table_data(data) == [{"name": "Alice"}] + data = {'data': {'users': [{'name': 'Alice'}]}} + assert extract_table_data(data) == [{'name': 'Alice'}] def test_single_object(self): - data = {"key": "value", "num": 42} + data = {'key': 'value', 'num': 42} assert extract_table_data(data) == [data] def test_empty_list(self): - assert extract_table_data([]) == [{"value": item} for item in []] + assert extract_table_data([]) == [{'value': item} for item in []] def test_non_dict_non_list(self): - assert extract_table_data("hello") == [] + assert extract_table_data('hello') == [] class TestParseJsonl: @@ -86,7 +95,7 @@ def test_basic_jsonl(self): text = '{"id": 1}\n{"id": 2}\n{"id": 3}' result = parse_jsonl(text) assert len(result) == 3 - assert result[0] == {"id": 1} + assert result[0] == {'id': 1} def test_empty_lines_skipped(self): text = '{"a": 1}\n\n{"a": 2}\n' @@ -98,6 +107,7 @@ def test_empty_input(self): def test_invalid_line_raises(self): import pytest + with pytest.raises(ValueError, match='line 2'): parse_jsonl('{"a": 1}\n{bad json}\n{"a": 3}') @@ -107,65 +117,158 @@ def test_mixed_objects(self): assert result[1]['name'] == 'Bob' -class TestFindCandidateArrays: - def test_single_array_at_root(self): - data = [{"id": 1}, {"id": 2}] - candidates = find_candidate_arrays(data) - assert len(candidates) == 1 - assert candidates[0]['path'] == '(root)' - assert candidates[0]['length'] == 2 - - def test_multiple_arrays(self): - data = {"users": [{"name": "A"}], "orders": [{"id": 1}, {"id": 2}]} - candidates = find_candidate_arrays(data) - assert len(candidates) == 2 - paths = [c['path'] for c in candidates] - assert 'users' in paths - assert 'orders' in paths - - def test_nested_array(self): - data = {"data": {"items": [{"x": 1}]}} - candidates = find_candidate_arrays(data) - assert len(candidates) == 1 - assert candidates[0]['path'] == 'data.items' - - def test_no_arrays(self): - data = {"a": 1, "b": "text"} - assert find_candidate_arrays(data) == [] - - class TestExtractByPath: def test_root_path(self): - data = [{"a": 1}] + data = [{'a': 1}] assert extract_by_path(data, '(root)') == data def test_nested_path(self): - data = {"data": {"items": [1, 2, 3]}} + data = {'data': {'items': [1, 2, 3]}} assert extract_by_path(data, 'data.items') == [1, 2, 3] def test_invalid_path(self): - data = {"a": 1} + data = {'a': 1} assert extract_by_path(data, 'b.c') is None def test_array_index_in_path(self): - data = {"data": [{"orders": [{"x": 1}, {"x": 2}]}, {"orders": []}]} - assert extract_by_path(data, 'data.0.orders') == [{"x": 1}, {"x": 2}] + data = {'data': [{'orders': [{'x': 1}, {'x': 2}]}, {'orders': []}]} + assert extract_by_path(data, 'data.0.orders') == [{'x': 1}, {'x': 2}] def test_array_index_out_of_range(self): - assert extract_by_path({"a": [1, 2]}, 'a.5') is None + assert extract_by_path({'a': [1, 2]}, 'a.5') is None def test_non_numeric_index_on_array(self): - assert extract_by_path({"a": [1, 2]}, 'a.foo') is None + assert extract_by_path({'a': [1, 2]}, 'a.foo') is None class TestGetAllColumns: def test_basic(self): - data = [{"a": 1, "b": 2}, {"b": 3, "c": 4}] - assert get_all_columns(data) == ["a", "b", "c"] + data = [{'a': 1, 'b': 2}, {'b': 3, 'c': 4}] + assert get_all_columns(data) == ['a', 'b', 'c'] def test_empty(self): assert get_all_columns([]) == [] def test_non_dict_rows_ignored(self): - data = [{"a": 1}, "not a dict", {"b": 2}] - assert get_all_columns(data) == ["a", "b"] + data = [{'a': 1}, 'not a dict', {'b': 2}] + assert get_all_columns(data) == ['a', 'b'] + + +class TestExtractTableDataDepthGuard: + """F8 - extract_table_data must not recurse without a bound.""" + + @staticmethod + def _nest(depth): + """Build a depth-N chain of dicts iteratively (no recursion in the test).""" + node = {'leaf': 'value'} + for _ in range(depth): + node = {'a': node} + return node + + def test_deep_nesting_does_not_raise(self): + result = extract_table_data(self._nest(1500)) + assert isinstance(result, list) + assert len(result) == 1 + + def test_stops_at_max_depth(self): + # With max_depth=3 the descent stops before reaching the array. + data = {'a': {'b': {'c': {'d': [{'x': 1}]}}}} + assert extract_table_data(data, max_depth=3) == [{'d': [{'x': 1}]}] + + def test_shallow_data_is_unaffected_by_the_guard(self): + data = {'data': {'users': [{'name': 'Alice'}]}} + assert extract_table_data(data) == [{'name': 'Alice'}] + + def test_non_dict_at_the_cap_yields_no_rows(self): + assert extract_table_data('scalar', _depth=99, max_depth=10) == [] + + +class TestPreviewTruncate: + """P2.2 / P5 - a capped COPY; the original is never touched.""" + + def test_long_string_is_capped(self): + row = {'a': 'x' * 1000} + result = preview_truncate(row) + assert len(result['a']) == PREVIEW_MAX_STRING + len(PREVIEW_TRUNCATION_SUFFIX) + assert result['a'].endswith(PREVIEW_TRUNCATION_SUFFIX) + + def test_short_string_is_untouched(self): + assert preview_truncate({'a': 'short'}) == {'a': 'short'} + + def test_every_column_survives(self): + # Capping row keys would leave the preview table disagreeing with its + # own header, so only the values inside a row are capped. + row = {f'col{i}': i for i in range(60)} + assert list(preview_truncate(row).keys()) == list(row.keys()) + + def test_nested_object_keys_are_capped(self): + row = {'meta': {f'k{i}': i for i in range(100)}} + meta = preview_truncate(row)['meta'] + assert len(meta) == PREVIEW_MAX_ITEMS + 1 + assert '80 more keys' in meta[PREVIEW_TRUNCATION_SUFFIX] + + def test_nested_array_items_are_capped(self): + row = {'tags': list(range(100))} + tags = preview_truncate(row)['tags'] + assert len(tags) == PREVIEW_MAX_ITEMS + 1 + assert tags[:PREVIEW_MAX_ITEMS] == list(range(PREVIEW_MAX_ITEMS)) + assert '80 more items' in tags[-1] + + def test_input_is_not_mutated(self): + nested = {'k': 'y' * 1000, 'items': list(range(100))} + row = {'meta': nested, 'plain': 'z' * 1000} + snapshot = json.dumps(row, sort_keys=True) + + preview_truncate(row) + + assert json.dumps(row, sort_keys=True) == snapshot + assert len(row['meta']['k']) == 1000 + assert row['meta'] is nested + + def test_depth_is_bounded(self): + node = {'leaf': 'v'} + for _ in range(1500): + node = {'a': node} + result = preview_truncate(node) + assert isinstance(result, dict) + + def test_numbers_and_booleans_pass_through(self): + row = {'n': 1, 'f': 2.5, 'b': True, 'null': None} + assert preview_truncate(row) == row + + +class TestFlattenRows: + """P8 - one pass must produce exactly what two passes produced.""" + + CASES = [ + [{'a': 1, 'b': 2}, {'b': 3, 'c': 4}], + [{'z': 1}, {'a': 2}, {'m': 3}], + [{'meta': {'age': 30}, 'name': 'Alice', 'scores': [1, 2]}], + [{'a': {'b': {'c': 1}}}, {'a': {'b': {'d': 2}}}], + [], + [{}], + ] + + def test_matches_the_previous_two_pass_result(self): + for rows in self.CASES: + expected_rows = [flatten_for_csv(row) for row in rows] + expected_columns = get_all_columns(expected_rows) + + actual_rows, actual_columns = flatten_rows(rows) + + assert actual_rows == expected_rows + # Column order must be byte-identical, not merely equivalent. + assert actual_columns == expected_columns + + def test_respects_max_depth(self): + rows = [{'a': {'b': {'c': {'d': 1}}}}] + flattened, columns = flatten_rows(rows, max_depth=2) + assert columns == ['a.b'] + assert isinstance(flattened[0]['a.b'], str) + + def test_non_dict_row_keeps_its_existing_empty_key_behavior(self): + # flatten_for_csv maps a bare scalar to {'': value}, so '' has always been + # a column here. Preserved deliberately: P8 is a refactor, not a fix. + flattened, columns = flatten_rows([{'a': 1}, 'not a dict']) + assert flattened == [{'a': 1}, {'': 'not a dict'}] + assert columns == ['', 'a'] diff --git a/tests/test_routes.py b/tests/test_routes.py index 5b22ae6..d05c8a0 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -1,7 +1,33 @@ """Tests for Flask routes.""" +import ast +import csv +import gzip +import importlib +import io import json -from unittest.mock import patch, MagicMock +import logging +import os +import pathlib +import re +import subprocess +import sys +import tempfile +from unittest.mock import MagicMock, patch + +import pytest +from flask import Response, request + +import config as config_module +from app import ( + check_fetch_timeout_headroom, + check_rate_limit_topology, + create_app, + worker_count_from_start_command, + worker_timeout_from_start_command, +) +from extensions import client_ip_key +from security import validate_url class TestIndexRoute: @@ -29,11 +55,14 @@ def test_returns_version(self, client, app): class TestProcessRoute: def test_paste_valid_json(self, client): - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': '[{"id": 1, "name": "Alice"}, {"id": 2, "name": "Bob"}]', - 'json_path': '(root)' - }) + response = client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': '[{"id": 1, "name": "Alice"}, {"id": 2, "name": "Bob"}]', + 'json_path': '(root)', + }, + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 @@ -41,54 +70,46 @@ def test_paste_valid_json(self, client): assert 'name' in data['columns'] def test_paste_invalid_json(self, client): - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': '{invalid json}' - }) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': '{invalid json}'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'error' in data def test_paste_empty(self, client): - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': '' - }) + response = client.post('/process', data={'input_method': 'paste', 'pasted_json': ''}) assert response.status_code == 400 def test_invalid_input_method(self, client): - response = client.post('/process', data={ - 'input_method': 'unknown' - }) + response = client.post('/process', data={'input_method': 'unknown'}) assert response.status_code == 400 def test_nested_json_object(self, client): - nested = json.dumps({"data": [{"x": 1}, {"x": 2}]}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': nested, - 'json_path': 'data' - }) + nested = json.dumps({'data': [{'x': 1}, {'x': 2}]}) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': nested, 'json_path': 'data'} + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 def test_no_path_returns_tree_payload(self, client): - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': '[{"id": 1}, {"id": 2}]' - }) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': '[{"id": 1}, {"id": 2}]'} + ) data = json.loads(response.data) assert data.get('needs_selection') is True - assert data['raw_json'] == [{"id": 1}, {"id": 2}] + assert data['raw_json'] == [{'id': 1}, {'id': 2}] def test_file_upload(self, client): import io - json_content = json.dumps([{"a": 1}]) + + json_content = json.dumps([{'a': 1}]) data = { 'input_method': 'file', 'json_path': '(root)', - 'json_file': (io.BytesIO(json_content.encode()), 'test.json') + 'json_file': (io.BytesIO(json_content.encode()), 'test.json'), } response = client.post('/process', data=data, content_type='multipart/form-data') result = json.loads(response.data) @@ -96,24 +117,28 @@ def test_file_upload(self, client): def test_jsonl_paste(self, client): jsonl_content = '{"id": 1, "name": "Alice"}\n{"id": 2, "name": "Bob"}' - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': jsonl_content, - 'data_format': 'jsonl', - 'json_path': '(root)' - }) + response = client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': jsonl_content, + 'data_format': 'jsonl', + 'json_path': '(root)', + }, + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 def test_jsonl_file_upload(self, client): import io + jsonl_content = '{"a": 1}\n{"a": 2}\n{"a": 3}' data = { 'input_method': 'file', 'data_format': 'jsonl', 'json_path': '(root)', - 'json_file': (io.BytesIO(jsonl_content.encode()), 'test.jsonl') + 'json_file': (io.BytesIO(jsonl_content.encode()), 'test.jsonl'), } response = client.post('/process', data=data, content_type='multipart/form-data') result = json.loads(response.data) @@ -122,12 +147,10 @@ def test_jsonl_file_upload(self, client): def test_preview_limit(self, client, app): app.config['PREVIEW_ROW_LIMIT'] = 5 - rows = json.dumps([{"id": i} for i in range(20)]) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': rows, - 'json_path': '(root)' - }) + rows = json.dumps([{'id': i} for i in range(20)]) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': rows, 'json_path': '(root)'} + ) data = json.loads(response.data) assert data['total_rows'] == 20 assert len(data['preview']) == 5 @@ -135,12 +158,12 @@ def test_preview_limit(self, client, app): class TestExportCsvRoute: def test_export_valid_data(self, client): - response = client.post('/export-csv', - data=json.dumps({ - 'csv_data': [{'a': 1, 'b': 2}, {'a': 3, 'b': 4}], - 'csv_columns': ['a', 'b'] - }), - content_type='application/json' + response = client.post( + '/export-csv', + data=json.dumps( + {'csv_data': [{'a': 1, 'b': 2}, {'a': 3, 'b': 4}], 'csv_columns': ['a', 'b']} + ), + content_type='application/json', ) assert response.status_code == 200 assert response.content_type.startswith('text/csv') @@ -148,99 +171,109 @@ def test_export_valid_data(self, client): assert 'a,b' in csv_text def test_export_empty_data(self, client): - response = client.post('/export-csv', - data=json.dumps({ - 'csv_data': [], - 'csv_columns': [] - }), - content_type='application/json' + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': [], 'csv_columns': []}), + content_type='application/json', + ) + assert response.status_code == 400 + + def test_non_dict_row_is_rejected_before_the_stream_starts(self, client): + # _stream_csv calls row.get(), and the generator body runs after the + # headers are already on the wire -- so without an up-front check this + # returned 200 and then raised AttributeError mid-body, leaving the + # client with a truncated file it had no way to recognize as an error. + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': ['x'], 'csv_columns': ['a']}), + content_type='application/json', + ) + assert response.status_code == 400 + assert response.get_json()['error'] == 'csv_data must be a list of objects' + # Reading the body must not raise: nothing was streamed. + assert b'AttributeError' not in response.get_data() + + def test_a_non_dict_row_after_valid_rows_is_rejected(self, client): + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': [{'a': 1}, None], 'csv_columns': ['a']}), + content_type='application/json', ) assert response.status_code == 400 class TestExportXlsxRoute: def test_export_xlsx(self, client): - response = client.post('/export-xlsx', - data=json.dumps({ - 'csv_data': [{'a': 1, 'b': 2}], - 'csv_columns': ['a', 'b'] - }), - content_type='application/json' + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'a': 1, 'b': 2}], 'csv_columns': ['a', 'b']}), + content_type='application/json', ) assert response.status_code == 200 assert 'spreadsheetml' in response.content_type def test_export_xlsx_empty(self, client): - response = client.post('/export-xlsx', + response = client.post( + '/export-xlsx', data=json.dumps({'csv_data': [], 'csv_columns': []}), - content_type='application/json' + content_type='application/json', ) assert response.status_code == 400 class TestPathSelection: def test_no_path_returns_raw_json_for_tree(self, client): - payload = {"users": [{"n": "A"}], "orders": [{"id": 1}]} - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': json.dumps(payload) - }) + payload = {'users': [{'n': 'A'}], 'orders': [{'id': 1}]} + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': json.dumps(payload)} + ) data = json.loads(response.data) assert data.get('needs_selection') is True assert data['raw_json'] == payload def test_path_selection(self, client): - multi = json.dumps({"users": [{"n": "A"}], "orders": [{"id": 1}, {"id": 2}]}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': multi, - 'json_path': 'orders' - }) + multi = json.dumps({'users': [{'n': 'A'}], 'orders': [{'id': 1}, {'id': 2}]}) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': multi, 'json_path': 'orders'} + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 def test_path_to_array_index_object(self, client): - payload = json.dumps({"data": [{"id": 1, "orders": [{"x": 1}, {"x": 2}]}]}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': payload, - 'json_path': 'data.0.orders' - }) + payload = json.dumps({'data': [{'id': 1, 'orders': [{'x': 1}, {'x': 2}]}]}) + response = client.post( + '/process', + data={'input_method': 'paste', 'pasted_json': payload, 'json_path': 'data.0.orders'}, + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 def test_path_to_single_object_becomes_one_row(self, client): - payload = json.dumps({"meta": {"version": 3, "name": "x"}}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': payload, - 'json_path': 'meta' - }) + payload = json.dumps({'meta': {'version': 3, 'name': 'x'}}) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': payload, 'json_path': 'meta'} + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 1 assert 'version' in data['columns'] def test_path_to_primitive_rejected(self, client): - payload = json.dumps({"a": 1}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': payload, - 'json_path': 'a' - }) + payload = json.dumps({'a': 1}) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': payload, 'json_path': 'a'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'primitive' in data['error'] def test_invalid_path_rejected(self, client): - payload = json.dumps({"a": {"b": 1}}) - response = client.post('/process', data={ - 'input_method': 'paste', - 'pasted_json': payload, - 'json_path': 'a.c' - }) + payload = json.dumps({'a': {'b': 1}}) + response = client.post( + '/process', data={'input_method': 'paste', 'pasted_json': payload, 'json_path': 'a.c'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'not found' in data['error'] @@ -253,6 +286,7 @@ def _mock_response(self, json_data, status_code=200): content = json.dumps(json_data).encode() mock_resp.iter_content.return_value = [content] mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp return mock_resp @patch('routes.requests.get') @@ -261,11 +295,14 @@ def test_api_fetch_success(self, mock_validate, mock_get, client): mock_validate.return_value = (True, None) mock_get.return_value = self._mock_response([{'id': 1}, {'id': 2}]) - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data', - 'json_path': '(root)' - }) + response = client.post( + '/process', + data={ + 'input_method': 'api', + 'api_url': 'https://api.example.com/data', + 'json_path': '(root)', + }, + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 @@ -277,13 +314,13 @@ def test_api_fetch_success(self, mock_validate, mock_get, client): @patch('routes.validate_url') def test_api_fetch_timeout(self, mock_validate, mock_get, client): import requests as req + mock_validate.return_value = (True, None) mock_get.side_effect = req.exceptions.Timeout('timed out') - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data' - }) + response = client.post( + '/process', data={'input_method': 'api', 'api_url': 'https://api.example.com/data'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'timed out' in data['error'].lower() @@ -296,12 +333,12 @@ def test_api_fetch_non_json(self, mock_validate, mock_get, client): mock_resp.status_code = 200 mock_resp.iter_content.return_value = [b'not json'] mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp mock_get.return_value = mock_resp - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data' - }) + response = client.post( + '/process', data={'input_method': 'api', 'api_url': 'https://api.example.com/data'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'not valid JSON' in data['error'] @@ -315,24 +352,26 @@ def test_api_fetch_max_size_exceeded(self, mock_validate, mock_get, client, app) mock_resp.status_code = 200 mock_resp.iter_content.return_value = [b'x' * 200] mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp mock_get.return_value = mock_resp - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data' - }) + response = client.post( + '/process', data={'input_method': 'api', 'api_url': 'https://api.example.com/data'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'exceeds maximum size' in data['error'] @patch('routes.validate_url') def test_api_fetch_ssrf_blocked(self, mock_validate, client): - mock_validate.return_value = (False, 'URLs pointing to private or internal networks are not allowed') + mock_validate.return_value = ( + False, + 'URLs pointing to private or internal networks are not allowed', + ) - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'http://169.254.169.254/metadata' - }) + response = client.post( + '/process', data={'input_method': 'api', 'api_url': 'http://169.254.169.254/metadata'} + ) assert response.status_code == 400 data = json.loads(response.data) assert 'private' in data['error'].lower() @@ -346,14 +385,18 @@ def test_api_fetch_jsonl(self, mock_validate, mock_get, client): content = b'{"id": 1, "name": "Alice"}\n{"id": 2, "name": "Bob"}' mock_resp.iter_content.return_value = [content] mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp mock_get.return_value = mock_resp - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data', - 'data_format': 'jsonl', - 'json_path': '(root)' - }) + response = client.post( + '/process', + data={ + 'input_method': 'api', + 'api_url': 'https://api.example.com/data', + 'data_format': 'jsonl', + 'json_path': '(root)', + }, + ) data = json.loads(response.data) assert data['success'] is True assert data['total_rows'] == 2 @@ -362,15 +405,15 @@ def test_api_fetch_jsonl(self, mock_validate, mock_get, client): @patch('routes.validate_url') def test_api_fetch_request_error_no_leak(self, mock_validate, mock_get, client): import requests as req + mock_validate.return_value = (True, None) mock_get.side_effect = req.exceptions.ConnectionError( 'Connection to secret-internal-host:8080 refused' ) - response = client.post('/process', data={ - 'input_method': 'api', - 'api_url': 'https://api.example.com/data' - }) + response = client.post( + '/process', data={'input_method': 'api', 'api_url': 'https://api.example.com/data'} + ) assert response.status_code == 400 data = json.loads(response.data) # Should NOT leak the internal connection details @@ -381,12 +424,10 @@ def test_api_fetch_request_error_no_leak(self, mock_validate, mock_get, client): class TestFileUploadEncoding: def test_non_utf8_file_returns_400(self, client): import io + # Latin-1 encoded content with bytes invalid in UTF-8 content = b'\xff\xfe This is not valid UTF-8' - data = { - 'input_method': 'file', - 'json_file': (io.BytesIO(content), 'test.json') - } + data = {'input_method': 'file', 'json_file': (io.BytesIO(content), 'test.json')} response = client.post('/process', data=data, content_type='multipart/form-data') assert response.status_code == 400 result = json.loads(response.data) @@ -395,15 +436,13 @@ def test_non_utf8_file_returns_400(self, client): class TestExportEdgeCases: def test_export_csv_no_json_body(self, client): - response = client.post('/export-csv', data='not json', - content_type='application/json') + response = client.post('/export-csv', data='not json', content_type='application/json') assert response.status_code == 400 data = json.loads(response.data) assert 'Invalid or missing' in data['error'] def test_export_xlsx_no_json_body(self, client): - response = client.post('/export-xlsx', data='not json', - content_type='application/json') + response = client.post('/export-xlsx', data='not json', content_type='application/json') assert response.status_code == 400 data = json.loads(response.data) assert 'Invalid or missing' in data['error'] @@ -433,3 +472,1813 @@ def test_xcontent_type_header(self, client): def test_referrer_policy(self, client): response = client.get('/') assert 'strict-origin' in response.headers['Referrer-Policy'] + + def test_permissions_policy(self, client): + response = client.get('/') + policy = response.headers['Permissions-Policy'] + assert 'camera=()' in policy + assert 'microphone=()' in policy + assert 'geolocation=()' in policy + + def test_cross_origin_headers(self, client): + response = client.get('/') + assert response.headers['Cross-Origin-Opener-Policy'] == 'same-origin' + assert response.headers['Cross-Origin-Resource-Policy'] == 'same-origin' + + def test_csp_hardening_directives(self, client): + directives = { + part.strip() for part in client.get('/').headers['Content-Security-Policy'].split(';') + } + assert "object-src 'none'" in directives + assert "base-uri 'self'" in directives + assert "frame-ancestors 'none'" in directives + assert "form-action 'self'" in directives + assert 'upgrade-insecure-requests' in directives + + def test_csp_still_allows_google_fonts_and_data_images(self, client): + csp = client.get('/').headers['Content-Security-Policy'] + assert 'https://fonts.googleapis.com' in csp + assert 'https://fonts.gstatic.com' in csp + assert 'data:' in csp + + def test_csp_has_no_malformed_directive(self, client): + """A missing separator would fuse two directives into one token.""" + csp = client.get('/').headers['Content-Security-Policy'] + parts = [part.strip() for part in csp.split(';') if part.strip()] + assert len(parts) == len(set(parts)) + for part in parts: + assert not part.startswith("'") + # Each directive starts with a bare directive name. + assert re.match(r'^[a-z-]+( |$)', part), part + + def test_no_hsts_on_plain_http(self, client): + response = client.get('/') + assert 'Strict-Transport-Security' not in response.headers + + def test_hsts_on_secure_request(self, client): + response = client.get('/', base_url='https://localhost') + assert response.headers['Strict-Transport-Security'] == ( + 'max-age=31536000; includeSubDomains' + ) + + +class TestFormulaInjection: + """F1 - CSV/XLSX formula injection (CWE-1236).""" + + DANGEROUS = ['=SUM(A1)', '@cmd', '+1', '-1', '\tlead', '\rlead', '\nlead'] + + def test_csv_prefixes_every_trigger(self, client): + rows = [{'v': value} for value in self.DANGEROUS] + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': rows, 'csv_columns': ['v']}), + content_type='application/json', + ) + assert response.status_code == 200 + + parsed = list(csv.reader(io.StringIO(response.data.decode('utf-8')))) + emitted = [row[0] for row in parsed[1:]] + assert emitted == ["'" + value for value in self.DANGEROUS] + + def test_csv_leaves_safe_values_alone(self, client): + response = client.post( + '/export-csv', + data=json.dumps( + { + 'csv_data': [{'a': 'plain', 'b': 5, 'c': 'user@example.com'}], + 'csv_columns': ['a', 'b', 'c'], + } + ), + content_type='application/json', + ) + parsed = list(csv.reader(io.StringIO(response.data.decode('utf-8')))) + assert parsed[1] == ['plain', '5', 'user@example.com'] + + def test_csv_sanitizes_column_headers(self, client): + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': [{'=EVIL()': 1}], 'csv_columns': ['=EVIL()']}), + content_type='application/json', + ) + parsed = list(csv.reader(io.StringIO(response.data.decode('utf-8')))) + assert parsed[0] == ["'=EVIL()"] + + def test_csv_serializes_containers(self, client): + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': [{'a': {'k': 'v'}}], 'csv_columns': ['a']}), + content_type='application/json', + ) + parsed = list(csv.reader(io.StringIO(response.data.decode('utf-8')))) + assert parsed[1] == ['{"k": "v"}'] + + def test_xlsx_writes_triggers_as_string_cells(self, client): + from openpyxl import load_workbook + + rows = [{'v': value} for value in self.DANGEROUS] + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': rows, 'csv_columns': ['v']}), + content_type='application/json', + ) + assert response.status_code == 200 + + ws = load_workbook(io.BytesIO(response.data)).active + for index, value in enumerate(self.DANGEROUS, start=2): + cell = ws.cell(row=index, column=1) + assert cell.data_type == 's', f'{value!r} was written as {cell.data_type}' + # XLSX carries an explicit type, so the text itself stays intact. + assert cell.value == value.replace('\r', '\n') + + def test_xlsx_sanitizes_column_headers(self, client): + from openpyxl import load_workbook + + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'=EVIL()': 1}], 'csv_columns': ['=EVIL()']}), + content_type='application/json', + ) + ws = load_workbook(io.BytesIO(response.data)).active + assert ws.cell(row=1, column=1).data_type == 's' + + def test_xlsx_keeps_numbers_numeric(self, client): + from openpyxl import load_workbook + + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'a': -1, 'b': 2.5}], 'csv_columns': ['a', 'b']}), + content_type='application/json', + ) + ws = load_workbook(io.BytesIO(response.data)).active + assert ws.cell(row=2, column=1).value == -1 + assert ws.cell(row=2, column=2).value == 2.5 + + +class TestApiFetchLogHygiene: + """F3/F9 - no URL component or token may reach the logs.""" + + SECRET = 'sup3rs3cr3t-token' + + def _post(self, client, url, **extra): + data = {'input_method': 'api', 'api_url': url} + data.update(extra) + return client.post('/process', data=data) + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_token_in_query_string_never_logged(self, mock_validate, mock_get, client, caplog): + import requests as req + + mock_validate.return_value = (True, None) + url = f'https://api.example.com/data?api_key={self.SECRET}' + # requests puts the whole URL in the exception message. + mock_get.side_effect = req.exceptions.ConnectionError( + f"HTTPSConnectionPool(host='api.example.com', port=443): " + f'Max retries exceeded with url: /data?api_key={self.SECRET}' + ) + + with caplog.at_level(logging.DEBUG): + response = self._post(client, url) + + assert response.status_code == 400 + assert json.loads(response.data)['error'] == 'API request failed' + + logged = '\n'.join(record.getMessage() for record in caplog.records) + assert self.SECRET not in logged + assert 'api.example.com' not in logged + assert '/data' not in logged + assert 'API request failed' in logged + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_token_in_path_never_logged(self, mock_validate, mock_get, client, caplog): + import requests as req + + mock_validate.return_value = (True, None) + url = f'https://api.example.com/v1/{self.SECRET}/data' + mock_get.side_effect = req.exceptions.ConnectionError( + f'Failed to establish a new connection to /v1/{self.SECRET}/data' + ) + + with caplog.at_level(logging.DEBUG): + response = self._post(client, url) + + assert response.status_code == 400 + logged = '\n'.join(record.getMessage() for record in caplog.records) + assert self.SECRET not in logged + assert 'v1' not in logged + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_query_param_auth_value_never_logged(self, mock_validate, mock_get, client, caplog): + import requests as req + + mock_validate.return_value = (True, None) + mock_get.side_effect = req.exceptions.ConnectionError('boom') + + with caplog.at_level(logging.DEBUG): + response = self._post( + client, + 'https://api.example.com/data', + auth_method='query_param', + query_param_name='api_key', + query_param_value=self.SECRET, + ) + + assert response.status_code == 400 + assert self.SECRET not in '\n'.join(r.getMessage() for r in caplog.records) + + +class TestApiFetchJsonlErrors: + """F9 - a malformed JSONL body from the API is a 400, not a logged 500.""" + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_malformed_jsonl_returns_400(self, mock_validate, mock_get, client, caplog): + mock_validate.return_value = (True, None) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.iter_content.return_value = [b'{"a": 1}\n{bad json}\n'] + mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp + mock_get.return_value = mock_resp + + with caplog.at_level(logging.DEBUG): + response = client.post( + '/process', + data={ + 'input_method': 'api', + 'api_url': 'https://api.example.com/data', + 'data_format': 'jsonl', + }, + ) + + assert response.status_code == 400 + assert json.loads(response.data)['error'] == 'API response is not valid JSONL' + + logged = '\n'.join(record.getMessage() for record in caplog.records) + assert 'Unexpected error' not in logged + assert 'bad json' not in logged + assert not [r for r in caplog.records if r.levelno >= logging.ERROR] + + +class TestOutboundHeaderAllowlist: + """F4 - the client supplies the outbound header NAME; only an allowlist passes.""" + + REJECTED = [ + 'Host', + 'host', + 'HOST', + 'Content-Length', + 'Transfer-Encoding', + 'Connection', + 'CoNnEcTiOn', + 'Proxy-Authorization', + 'PROXY-AUTHORIZATION', + 'Cookie', + 'X-CSRF-Token', + ' Host ', + 'Host\t', + 'X Api Key', + 'X-Api-Key:', + '', + ] + + def _fetch(self, client, header_name): + return client.post( + '/process', + data={ + 'input_method': 'api', + 'api_url': 'https://api.example.com/data', + 'auth_method': 'api_key', + 'api_key_header': header_name, + 'api_key': 'secret', + 'json_path': '(root)', + }, + ) + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_reserved_and_hop_by_hop_names_rejected(self, mock_validate, mock_get, client): + mock_validate.return_value = (True, None) + + for name in self.REJECTED: + response = self._fetch(client, name) + assert response.status_code == 400, f'{name!r} was accepted' + assert 'not permitted' in json.loads(response.data)['error'] + + # Nothing was ever sent upstream. + assert mock_get.call_count == 0 + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_permitted_header_in_unusual_case_is_forwarded(self, mock_validate, mock_get, client): + mock_validate.return_value = (True, None) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.iter_content.return_value = [b'[{"id": 1}]'] + mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp + mock_get.return_value = mock_resp + + response = self._fetch(client, ' x-API-kEy ') + assert json.loads(response.data)['success'] is True + + _, kwargs = mock_get.call_args + assert kwargs['headers'] == {'x-API-kEy': 'secret'} + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_default_header_still_works(self, mock_validate, mock_get, client): + mock_validate.return_value = (True, None) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.iter_content.return_value = [b'[{"id": 1}]'] + mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp + mock_get.return_value = mock_resp + + response = self._fetch(client, 'X-API-Key') + assert json.loads(response.data)['success'] is True + _, kwargs = mock_get.call_args + assert kwargs['headers'] == {'X-API-Key': 'secret'} + + +@pytest.fixture +def fresh_config(monkeypatch): + """ + Rebuild config.Config from the current environment. + + Config holds class attributes evaluated at import time, so a monkeypatched + env var only takes effect after a reload. F7 rules out constructing + Config(...) -- it is a class, not a constructor. + """ + + def build(**env): + for name, value in env.items(): + if value is None: + monkeypatch.delenv(name, raising=False) + else: + monkeypatch.setenv(name, value) + return importlib.reload(config_module).Config + + yield build + # Leave the module holding the pristine values for every later test. + monkeypatch.undo() + importlib.reload(config_module) + + +class TestProductionSecretKey: + """F7 - the dev SECRET_KEY must not survive into production.""" + + def test_production_with_default_key_refuses_to_start(self, fresh_config): + cfg = fresh_config(APP_ENV='production', SECRET_KEY=None) + with pytest.raises(RuntimeError, match='SECRET_KEY must be set'): + create_app(cfg) + + def test_production_with_empty_key_refuses_to_start(self, fresh_config): + # A misconfigured secrets manager produces '' rather than "unset". + cfg = fresh_config(APP_ENV='production', SECRET_KEY='') + with pytest.raises(RuntimeError, match='SECRET_KEY must be set'): + create_app(cfg) + + def test_production_with_real_key_starts(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='a-real-random-value', + # A production app must also declare its topology (2.10). + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + ) + app = create_app(cfg) + assert app.config['SECRET_KEY'] == 'a-real-random-value' + + def test_local_run_with_default_key_starts(self, fresh_config): + # `python app.py` has DEBUG False, so gating on `not DEBUG` would have + # blocked the documented local run. + cfg = fresh_config(APP_ENV=None, SECRET_KEY=None, FLASK_DEBUG=None) + assert cfg.DEBUG is False + app = create_app(cfg) + assert app.config['SECRET_KEY'] == config_module.DEV_SECRET_KEY + + def test_second_spelling_is_not_a_production_signal(self, fresh_config): + # Accepting PRODUCTION=true as well would let a deployment pass this gate + # while SESSION_COOKIE_SECURE (F16) stayed off. + cfg = fresh_config(APP_ENV=None, PRODUCTION='true', SECRET_KEY=None) + assert config_module.is_production() is False + create_app(cfg) + + def test_app_env_is_case_and_space_insensitive(self, fresh_config): + fresh_config(APP_ENV=' Production ') + assert config_module.is_production() is True + + +class TestIntegerConfigValidation: + """F7 - a mistyped integer setting must name the variable, not raise ValueError.""" + + INT_SETTINGS = [ + 'MAX_UPLOAD_SIZE', + 'PREVIEW_ROW_LIMIT', + 'API_FETCH_TIMEOUT', + 'API_FETCH_MAX_RESPONSE', + 'FLATTEN_MAX_DEPTH', + 'API_DNS_TIMEOUT', + 'API_DNS_MAX_WORKERS', + 'API_DNS_ADMISSION_TIMEOUT', + ] + + def test_each_integer_setting_reports_a_clear_error(self, fresh_config): + for name in self.INT_SETTINGS: + with pytest.raises(RuntimeError, match=f'{name} must be an integer'): + fresh_config(**{name: 'abc'}) + # Undo before the next iteration so errors do not stack. + fresh_config(**{name: None}) + + def test_blank_value_falls_back_to_the_default(self, fresh_config): + cfg = fresh_config(PREVIEW_ROW_LIMIT=' ') + assert cfg.PREVIEW_ROW_LIMIT == 25 + + def test_valid_value_is_applied(self, fresh_config): + cfg = fresh_config(PREVIEW_ROW_LIMIT=' 7 ') + assert cfg.PREVIEW_ROW_LIMIT == 7 + + def test_port_allowlist_reports_a_clear_error(self, fresh_config): + with pytest.raises(RuntimeError, match='API_ALLOWED_PORTS must be a comma-separated'): + fresh_config(API_ALLOWED_PORTS='80,https') + fresh_config(API_ALLOWED_PORTS=None) + + def test_port_allowlist_is_parsed(self, fresh_config): + cfg = fresh_config(API_ALLOWED_PORTS=' 80 , 8443 ') + assert sorted(cfg.API_ALLOWED_PORTS) == [80, 8443] + + +class TestRecursionDepth: + """F8 - a pathologically nested document is a 400, never a 500.""" + + @staticmethod + def _deep_json(depth): + return '{"a":' * depth + '1' + '}' * depth + + def test_deeply_nested_paste_returns_400(self, client, caplog): + with caplog.at_level(logging.DEBUG): + response = client.post( + '/process', + data={'input_method': 'paste', 'pasted_json': self._deep_json(1500)}, + ) + assert response.status_code == 400 + assert json.loads(response.data)['error'] == 'JSON nesting too deep' + assert not [r for r in caplog.records if r.levelno >= logging.ERROR] + + def test_deeply_nested_upload_returns_400(self, client): + response = client.post( + '/process', + data={ + 'input_method': 'file', + 'json_file': (io.BytesIO(self._deep_json(1500).encode()), 'deep.json'), + }, + content_type='multipart/form-data', + ) + assert response.status_code == 400 + assert json.loads(response.data)['error'] == 'JSON nesting too deep' + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_deeply_nested_api_response_returns_400(self, mock_validate, mock_get, client): + mock_validate.return_value = (True, None) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.iter_content.return_value = [self._deep_json(1500).encode()] + mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp + mock_get.return_value = mock_resp + + response = client.post( + '/process', + data={'input_method': 'api', 'api_url': 'https://api.example.com/data'}, + ) + assert response.status_code == 400 + + def test_moderately_nested_document_still_works(self, client): + payload = json.dumps({'rows': [{'id': 1}, {'id': 2}]}) + response = client.post( + '/process', + data={'input_method': 'paste', 'pasted_json': payload, 'json_path': 'rows'}, + ) + assert json.loads(response.data)['total_rows'] == 2 + + +class TestProxyAwareRateLimiting: + """F12 / D3 - X-Forwarded-For is honored only under TRUST_PROXY=1.""" + + @staticmethod + def _app_capturing_remote_addr(cfg): + app = create_app(cfg) + app.config['TESTING'] = True + app.config['WTF_CSRF_ENABLED'] = False + seen = {} + + @app.before_request + def _capture(): + seen['remote_addr'] = request.remote_addr + seen['key'] = client_ip_key() + seen['is_secure'] = request.is_secure + + return app, seen + + def test_forwarded_for_ignored_by_default(self, fresh_config): + cfg = fresh_config(TRUST_PROXY=None) + app, seen = self._app_capturing_remote_addr(cfg) + + app.test_client().get( + '/health', + headers={'X-Forwarded-For': '9.9.9.9', 'X-Forwarded-Proto': 'https'}, + environ_base={'REMOTE_ADDR': '10.0.0.5'}, + ) + + assert seen['remote_addr'] == '10.0.0.5' + assert seen['key'] == '10.0.0.5' + assert seen['is_secure'] is False + + def test_forwarded_for_used_when_trusted(self, fresh_config): + cfg = fresh_config(TRUST_PROXY='1') + app, seen = self._app_capturing_remote_addr(cfg) + + app.test_client().get( + '/health', + headers={'X-Forwarded-For': '9.9.9.9', 'X-Forwarded-Proto': 'https'}, + environ_base={'REMOTE_ADDR': '10.0.0.5'}, + ) + + assert seen['remote_addr'] == '9.9.9.9' + assert seen['key'] == '9.9.9.9' + # x_proto=1 also makes is_secure correct behind a TLS-terminating proxy, + # which HSTS (1.5) and the Secure cookie (1.14) depend on. + assert seen['is_secure'] is True + + def test_only_one_hop_is_trusted(self, fresh_config): + """ + A client that prepends its own hop must not choose its bucket. + + With x_for=1 ProxyFix takes the LAST entry -- the hop our single trusted + proxy actually appended -- so the forged leading entry is ignored. + """ + cfg = fresh_config(TRUST_PROXY='1') + app, seen = self._app_capturing_remote_addr(cfg) + + app.test_client().get( + '/health', + headers={'X-Forwarded-For': '1.1.1.1, 2.2.2.2, 3.3.3.3'}, + environ_base={'REMOTE_ADDR': '10.0.0.5'}, + ) + + assert seen['remote_addr'] == '3.3.3.3' + + def test_different_clients_get_different_buckets(self, fresh_config): + cfg = fresh_config(TRUST_PROXY='1') + app, seen = self._app_capturing_remote_addr(cfg) + client = app.test_client() + + keys = [] + for ip in ('9.9.9.9', '8.8.8.8'): + client.get( + '/health', + headers={'X-Forwarded-For': ip}, + environ_base={'REMOTE_ADDR': '10.0.0.5'}, + ) + keys.append(seen['key']) + + assert keys == ['9.9.9.9', '8.8.8.8'] + + +class TestJsonErrorHandlers: + """F10 - every error response is JSON, including the framework's own.""" + + def test_oversized_request_returns_json_413(self, fresh_config): + cfg = fresh_config(MAX_UPLOAD_SIZE=str(1024 * 1024)) + app = create_app(cfg) + app.config['WTF_CSRF_ENABLED'] = False + + response = app.test_client().post( + '/process', + data={'input_method': 'paste', 'pasted_json': 'x' * (2 * 1024 * 1024)}, + ) + + assert response.status_code == 413 + assert response.content_type.startswith('application/json') + assert json.loads(response.data)['error'] == 'Request too large (max 1MB)' + + def test_unknown_route_returns_json_404(self, client): + response = client.get('/no-such-route') + assert response.status_code == 404 + assert response.content_type.startswith('application/json') + assert 'error' in json.loads(response.data) + + def test_internal_error_returns_json_500(self, fresh_config): + cfg = fresh_config() + app = create_app(cfg) + app.config['WTF_CSRF_ENABLED'] = False + # TESTING would re-raise instead of routing to the handler. + app.config['PROPAGATE_EXCEPTIONS'] = False + + @app.route('/boom') + def _boom(): + raise RuntimeError('kaboom') + + response = app.test_client().get('/boom') + assert response.status_code == 500 + assert response.content_type.startswith('application/json') + assert json.loads(response.data)['error'] == 'An internal error occurred' + assert b'kaboom' not in response.data + + +class TestNoStoreCacheControl: + """F11 - data-bearing responses must not be retained by any cache.""" + + def test_health_is_no_store(self, client): + assert client.get('/health').headers['Cache-Control'] == 'no-store' + + def test_process_is_no_store(self, client): + response = client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': '[{"a": 1}]', + 'json_path': '(root)', + }, + ) + assert response.headers['Cache-Control'] == 'no-store' + + def test_export_csv_is_no_store(self, client): + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': [{'a': 1}], 'csv_columns': ['a']}), + content_type='application/json', + ) + assert response.headers['Cache-Control'] == 'no-store' + + def test_export_xlsx_is_no_store(self, client): + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'a': 1}], 'csv_columns': ['a']}), + content_type='application/json', + ) + assert response.headers['Cache-Control'] == 'no-store' + + def test_index_page_is_still_cacheable(self, client): + assert client.get('/').headers.get('Cache-Control') != 'no-store' + + +class TestUploadValidation: + """F13 - the server, not just the file input's accept attribute.""" + + def _upload(self, client, filename, content_type=None, body=b'[{"a": 1}]'): + data = { + 'input_method': 'file', + 'json_path': '(root)', + 'json_file': (io.BytesIO(body), filename, content_type) + if content_type is not None + else (io.BytesIO(body), filename), + } + return client.post('/process', data=data, content_type='multipart/form-data') + + def test_rejects_unexpected_extensions(self, client): + for filename in ('evil.txt', 'evil.exe', 'evil', 'evil.json.png', 'evil.csv'): + response = self._upload(client, filename) + assert response.status_code == 400, filename + assert '.json or .jsonl' in json.loads(response.data)['error'] + + def test_rejects_unexpected_content_type(self, client): + response = self._upload(client, 'data.json', content_type='image/png') + assert response.status_code == 400 + assert 'Unsupported content type' in json.loads(response.data)['error'] + + def test_accepts_json_and_jsonl(self, client): + assert json.loads(self._upload(client, 'data.json').data)['success'] is True + assert json.loads(self._upload(client, 'DATA.JSON').data)['success'] is True + + def test_accepts_the_octet_stream_browsers_send_for_jsonl(self, client): + response = self._upload( + client, + 'data.jsonl', + content_type='application/octet-stream', + body=b'{"a": 1}', + ) + assert response.status_code == 200 + + +class TestCookieHardening: + """F16 - explicit cookie flags, Secure tied to APP_ENV=production.""" + + @staticmethod + def _set_cookie(app): + app.config['TESTING'] = True + # The index page calls csrf_token(), which writes to the session. + response = app.test_client().get('/', base_url='https://localhost') + return response.headers.get('Set-Cookie', '') + + def test_local_run_flags(self, fresh_config): + cfg = fresh_config(APP_ENV=None) + assert cfg.SESSION_COOKIE_HTTPONLY is True + assert cfg.SESSION_COOKIE_SAMESITE == 'Lax' + assert cfg.SESSION_COOKIE_SECURE is False + + cookie = self._set_cookie(create_app(cfg)) + assert 'HttpOnly' in cookie + assert 'SameSite=Lax' in cookie + assert 'Secure' not in cookie + + def test_production_sets_secure(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='a-real-random-value', + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + ) + assert cfg.SESSION_COOKIE_SECURE is True + + cookie = self._set_cookie(create_app(cfg)) + assert 'Secure' in cookie + assert 'HttpOnly' in cookie + assert 'SameSite=Lax' in cookie + + +class TestHealthVersionGate: + """F15 - version is returned by default; the gate only lets operators opt out.""" + + def test_version_present_by_default(self, fresh_config): + cfg = fresh_config(HEALTH_REVEAL_VERSION=None) + assert cfg.HEALTH_REVEAL_VERSION is True + + app = create_app(cfg) + data = json.loads(app.test_client().get('/health').data) + assert data['status'] == 'ok' + assert data['version'] == cfg.APP_VERSION + + def test_version_hidden_when_disabled(self, fresh_config): + cfg = fresh_config(HEALTH_REVEAL_VERSION='0') + assert cfg.HEALTH_REVEAL_VERSION is False + + app = create_app(cfg) + data = json.loads(app.test_client().get('/health').data) + assert data == {'status': 'ok'} + + +class TestGzipCompression: + """P1 - compress large text/JSON bodies, and nothing else.""" + + @staticmethod + def _big_payload(rows=400): + return json.dumps([{'id': i, 'name': f'user-{i}', 'note': 'x' * 60} for i in range(rows)]) + + def _process(self, client, **headers): + return client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': self._big_payload(), + 'json_path': '(root)', + }, + headers=headers, + ) + + def test_large_json_is_compressed(self, client): + response = self._process(client, **{'Accept-Encoding': 'gzip, deflate'}) + + assert response.headers['Content-Encoding'] == 'gzip' + assert 'Accept-Encoding' in response.headers['Vary'] + + raw = response.get_data() + decompressed = gzip.decompress(raw) + assert json.loads(decompressed)['total_rows'] == 400 + assert len(raw) < len(decompressed) + # Content-Length must describe what is actually on the wire. + assert int(response.headers['Content-Length']) == len(raw) + + def test_not_compressed_without_accept_encoding(self, client): + response = self._process(client, **{'Accept-Encoding': 'identity'}) + assert 'Content-Encoding' not in response.headers + assert 'Accept-Encoding' in response.headers['Vary'] + assert json.loads(response.data)['total_rows'] == 400 + + def test_explicit_gzip_refusal_is_honored(self, client): + # `gzip;q=0` names gzip only to reject it. A substring test on the raw + # header read that as permission and compressed anyway. + response = self._process(client, **{'Accept-Encoding': 'gzip;q=0'}) + assert 'Content-Encoding' not in response.headers + assert json.loads(response.data)['total_rows'] == 400 + + def test_refusal_among_other_encodings_is_honored(self, client): + response = self._process(client, **{'Accept-Encoding': 'br, gzip;q=0'}) + assert 'Content-Encoding' not in response.headers + + def test_positive_quality_still_compresses(self, client): + response = self._process(client, **{'Accept-Encoding': 'gzip;q=0.5'}) + assert response.headers['Content-Encoding'] == 'gzip' + + def test_wildcard_accepts_gzip(self, client): + # RFC 9110 12.5.3: `*` matches any encoding not otherwise named. + response = self._process(client, **{'Accept-Encoding': '*'}) + assert response.headers['Content-Encoding'] == 'gzip' + + def test_small_response_is_not_compressed(self, client): + response = client.get('/health', headers={'Accept-Encoding': 'gzip'}) + assert 'Content-Encoding' not in response.headers + assert json.loads(response.data)['status'] == 'ok' + + def test_vary_is_not_duplicated(self, client): + response = self._process(client, **{'Accept-Encoding': 'gzip'}) + varies = [v.strip().lower() for v in response.headers.get_all('Vary')] + assert varies.count('accept-encoding') == 1 + + def test_head_request_is_left_alone(self, client): + response = client.head('/', headers={'Accept-Encoding': 'gzip'}) + assert 'Content-Encoding' not in response.headers + + def test_already_encoded_body_is_left_alone(self, app): + app.config['PROPAGATE_EXCEPTIONS'] = False + + @app.route('/pre-encoded') + def _pre_encoded(): + payload = gzip.compress(b'{"already": "' + b'x' * 5000 + b'"}') + return Response( + payload, + mimetype='application/json', + headers={'Content-Encoding': 'gzip'}, + ) + + response = app.test_client().get('/pre-encoded', headers={'Accept-Encoding': 'gzip'}) + # Not double-compressed: one gzip layer decodes to the original JSON. + assert response.headers['Content-Encoding'] == 'gzip' + assert gzip.decompress(response.get_data()).startswith(b'{"already"') + + def test_streamed_response_is_left_alone(self, app): + @app.route('/streamed') + def _streamed(): + def generate(): + for index in range(500): + yield f'line {index} ' + 'y' * 40 + '\n' + + return Response(generate(), mimetype='text/plain') + + response = app.test_client().get('/streamed', headers={'Accept-Encoding': 'gzip'}) + assert 'Content-Encoding' not in response.headers + assert response.get_data().startswith(b'line 0 ') + + def test_binary_export_is_not_compressed(self, client): + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'a': 'x' * 100}] * 50, 'csv_columns': ['a']}), + content_type='application/json', + headers={'Accept-Encoding': 'gzip'}, + ) + # XLSX is a zip container; re-compressing it wastes CPU for nothing. + assert 'Content-Encoding' not in response.headers + + +class TestStaticAssetCaching: + """P6 - static assets are cacheable, and their URLs are version-busted.""" + + def test_static_assets_carry_a_max_age(self, client): + response = client.get('/static/css/style.css') + assert response.status_code == 200 + assert 'max-age=86400' in response.headers['Cache-Control'] + + def test_asset_urls_are_version_busted(self, client, app): + html = client.get('/').data.decode('utf-8') + version = app.config['APP_VERSION'] + assert f'css/style.css?v={version}' in html + assert f'js/app.js?v={version}' in html + + def test_max_age_is_configurable(self, fresh_config): + cfg = fresh_config(STATIC_MAX_AGE='60') + app = create_app(cfg) + response = app.test_client().get('/static/js/app.js') + assert 'max-age=60' in response.headers['Cache-Control'] + + +class TestStreamingCsvExport: + """P3 - CSV is generator-streamed and stays uncapped.""" + + def test_response_is_streamed(self, app): + client = app.test_client() + response = client.post( + '/export-csv', + data=json.dumps( + { + 'csv_data': [{'a': i, 'b': 'x' * 20} for i in range(2000)], + 'csv_columns': ['a', 'b'], + } + ), + content_type='application/json', + ) + assert response.status_code == 200 + # No Content-Length: the body is produced as it is written. + assert 'Content-Length' not in response.headers + rows = list(csv.reader(io.StringIO(response.get_data(as_text=True)))) + assert rows[0] == ['a', 'b'] + assert len(rows) == 2001 + + def test_csv_is_not_capped_by_the_xlsx_budget(self, app): + app.config['MAX_EXPORT_CELLS'] = 10 + response = app.test_client().post( + '/export-csv', + data=json.dumps({'csv_data': [{'a': i} for i in range(200)], 'csv_columns': ['a']}), + content_type='application/json', + ) + assert response.status_code == 200 + assert len(response.get_data(as_text=True).strip().splitlines()) == 201 + + def test_chunk_boundary_does_not_lose_or_duplicate_rows(self, app): + # Exercise exactly the flush boundary of CSV_STREAM_CHUNK_ROWS. + from routes import CSV_STREAM_CHUNK_ROWS + + for count in ( + CSV_STREAM_CHUNK_ROWS - 1, + CSV_STREAM_CHUNK_ROWS, + CSV_STREAM_CHUNK_ROWS + 1, + ): + response = app.test_client().post( + '/export-csv', + data=json.dumps( + {'csv_data': [{'a': i} for i in range(count)], 'csv_columns': ['a']} + ), + content_type='application/json', + ) + rows = list(csv.reader(io.StringIO(response.get_data(as_text=True)))) + assert [r[0] for r in rows[1:]] == [str(i) for i in range(count)], count + + +class TestXlsxExportBudget: + """P3 / D6 - the XLSX guard is on by default, budgeted in cells, never silent.""" + + def test_process_advertises_the_budget(self, client, app): + app.config['MAX_EXPORT_CELLS'] = 1000 + response = client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': json.dumps([{'a': i, 'b': i} for i in range(5)]), + 'json_path': '(root)', + }, + ) + data = json.loads(response.data) + assert data['total_rows'] == 5 + assert data['total_cells'] == 10 + assert data['max_export_cells'] == 1000 + + def test_existing_keys_are_unchanged(self, client): + response = client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': '[{"a": 1}]', + 'json_path': '(root)', + }, + ) + data = json.loads(response.data) + for key in ('success', 'columns', 'preview', 'total_rows', 'csv_data', 'csv_columns'): + assert key in data + assert data['total_rows'] == 1 + assert data['columns'] == ['a'] + + def test_oversized_export_is_refused_not_truncated(self, app): + app.config['MAX_EXPORT_CELLS'] = 10 + response = app.test_client().post( + '/export-xlsx', + data=json.dumps( + {'csv_data': [{'a': i, 'b': i} for i in range(20)], 'csv_columns': ['a', 'b']} + ), + content_type='application/json', + ) + assert response.status_code == 400 + error = json.loads(response.data)['error'] + assert '40 cells' in error + assert 'limit of 10' in error + assert 'CSV or TSV' in error + + def test_export_at_the_limit_succeeds(self, app): + app.config['MAX_EXPORT_CELLS'] = 40 + response = app.test_client().post( + '/export-xlsx', + data=json.dumps( + {'csv_data': [{'a': i, 'b': i} for i in range(20)], 'csv_columns': ['a', 'b']} + ), + content_type='application/json', + ) + assert response.status_code == 200 + + def test_zero_disables_the_guard(self, app): + app.config['MAX_EXPORT_CELLS'] = 0 + response = app.test_client().post( + '/export-xlsx', + data=json.dumps({'csv_data': [{'a': i} for i in range(50)], 'csv_columns': ['a']}), + content_type='application/json', + ) + assert response.status_code == 200 + + def test_guard_is_enabled_by_default(self, app): + assert app.config['MAX_EXPORT_CELLS'] > 0 + + def test_export_writes_no_files(self, app, tmp_path, monkeypatch): + """ + D6 / AGENTS.md: no disk writes of payloads. + + openpyxl's write_only mode and a rolled-over SpooledTemporaryFile both + put payload bytes in the OS temp directory. Point every temp mechanism at + an empty directory and assert nothing lands there. + """ + for var in ('TMPDIR', 'TEMP', 'TMP'): + monkeypatch.setenv(var, str(tmp_path)) + monkeypatch.setattr(tempfile, 'tempdir', str(tmp_path)) + + response = app.test_client().post( + '/export-xlsx', + data=json.dumps( + { + 'csv_data': [{'a': f'value-{i}', 'b': i} for i in range(3000)], + 'csv_columns': ['a', 'b'], + } + ), + content_type='application/json', + ) + assert response.status_code == 200 + assert len(response.get_data()) > 0 + assert list(tmp_path.iterdir()) == [] + + +class TestPreviewTruncationDoesNotAffectExports: + """P2.2 / P5 - the preview is capped; csv_data and exports are not.""" + + LONG = 'L' * 2000 + + def _process(self, client): + payload = json.dumps( + [{'text': self.LONG, 'items': list(range(100)), 'meta': {'a': self.LONG}}] + ) + return json.loads( + client.post( + '/process', + data={ + 'input_method': 'paste', + 'pasted_json': payload, + 'json_path': '(root)', + }, + ).data + ) + + def test_preview_is_truncated(self, client): + preview = self._process(client)['preview'][0] + assert preview['text'].endswith('… (truncated)') + assert len(preview['text']) < len(self.LONG) + assert len(preview['items']) == 21 + + def test_csv_data_keeps_full_fidelity(self, client): + csv_data = self._process(client)['csv_data'][0] + assert csv_data['text'] == self.LONG + assert csv_data['meta.a'] == self.LONG + assert json.loads(csv_data['items']) == list(range(100)) + + def test_server_csv_export_is_untruncated(self, client): + data = self._process(client) + response = client.post( + '/export-csv', + data=json.dumps({'csv_data': data['csv_data'], 'csv_columns': data['csv_columns']}), + content_type='application/json', + ) + rows = list(csv.reader(io.StringIO(response.get_data(as_text=True)))) + assert self.LONG in rows[1] + assert '… (truncated)' not in response.get_data(as_text=True) + + def test_xlsx_export_is_untruncated(self, client): + from openpyxl import load_workbook + + data = self._process(client) + response = client.post( + '/export-xlsx', + data=json.dumps({'csv_data': data['csv_data'], 'csv_columns': data['csv_columns']}), + content_type='application/json', + ) + ws = load_workbook(io.BytesIO(response.data)).active + values = [cell.value for cell in ws[2]] + assert self.LONG in values + + +class TestApiFetchDecodesWithoutAnExtraCopy: + """P12 - the streamed bytearray is decoded directly.""" + + @patch('routes.requests.get') + @patch('routes.validate_url') + def test_bytearray_is_decoded_in_place(self, mock_validate, mock_get, client): + mock_validate.return_value = (True, None) + mock_resp = MagicMock() + mock_resp.status_code = 200 + # Several chunks, plus a multi-byte character split across none of them, + # to prove the accumulated bytearray is what gets decoded. + mock_resp.iter_content.return_value = [ + b'[{"name": "Zo', + 'ë'.encode(), + b'"}]', + ] + mock_resp.raise_for_status.return_value = None + mock_resp.__enter__.return_value = mock_resp + mock_get.return_value = mock_resp + + response = client.post( + '/process', + data={ + 'input_method': 'api', + 'api_url': 'https://api.example.com/data', + 'json_path': '(root)', + }, + ) + data = json.loads(response.data) + assert data['success'] is True + assert data['csv_data'][0]['name'] == 'Zoë' + + +class TestRateLimitTopologyGuard: + """ + 2.10 / F12 - memory:// counters are process-local, so the guard must fail + closed rather than let a multi-worker deployment silently enforce N x the + configured limit. + """ + + def test_memory_storage_counters_are_not_shared(self): + """ + The multiplier is real, not theoretical. + + Two limiter storages on memory:// do not see each other's hits, which is + exactly what happens across gunicorn workers and across replicas. + """ + from limits.storage import storage_from_string + + first = storage_from_string('memory://') + second = storage_from_string('memory://') + + for _ in range(5): + first.incr('shared-key', 60) + + assert first.get('shared-key') == 5 + assert second.get('shared-key') == 0 + + def test_storage_uri_is_configurable_at_all(self, fresh_config): + # config.py hardcoded 'memory://' before v1.2, so no deployment could set + # shared storage even if it wanted to (2.10a). + cfg = fresh_config(RATELIMIT_STORAGE_URI='redis://localhost:6379/0') + assert cfg.RATELIMIT_STORAGE_URI == 'redis://localhost:6379/0' + + def test_default_topology_starts_clean(self, fresh_config, caplog): + cfg = fresh_config(APP_ENV=None, WEB_CONCURRENCY=None, APP_REPLICAS=None) + with caplog.at_level(logging.WARNING): + create_app(cfg) + assert 'Rate-limit topology' not in caplog.text + + def test_multi_worker_on_memory_storage_warns_outside_production(self, fresh_config, caplog): + cfg = fresh_config(APP_ENV=None, WEB_CONCURRENCY='4', APP_REPLICAS='1') + with caplog.at_level(logging.WARNING): + create_app(cfg) + assert 'process-local' in caplog.text + assert '4 x 1' in caplog.text + + def test_multi_worker_on_memory_storage_raises_in_production(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='4', + APP_REPLICAS='1', + ) + with pytest.raises(RuntimeError, match='process-local'): + create_app(cfg) + + def test_multi_replica_on_memory_storage_raises_in_production(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='1', + APP_REPLICAS='3', + ) + with pytest.raises(RuntimeError, match='1 x 3'): + create_app(cfg) + + def test_missing_declaration_raises_in_production(self, fresh_config): + cfg = fresh_config(APP_ENV='production', SECRET_KEY='k', WEB_CONCURRENCY=None) + with pytest.raises(RuntimeError, match='WEB_CONCURRENCY is not declared'): + create_app(cfg) + + def test_missing_replica_declaration_raises_in_production(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', SECRET_KEY='k', WEB_CONCURRENCY='1', APP_REPLICAS=None + ) + with pytest.raises(RuntimeError, match='APP_REPLICAS is not declared'): + create_app(cfg) + + def test_shared_storage_allows_a_declared_multi_worker_topology(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='4', + APP_REPLICAS='2', + RATELIMIT_STORAGE_URI='redis://localhost:6379/0', + ) + # Constructed, not connected: Flask-Limiter dials Redis lazily. + app = create_app(cfg) + assert app.config['RATELIMIT_STORAGE_URI'].startswith('redis://') + + def test_start_command_contradicting_the_declaration_raises(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', SECRET_KEY='k', WEB_CONCURRENCY='1', APP_REPLICAS='1' + ) + app = create_app(cfg) + argv = ['/usr/bin/gunicorn', 'app:create_app()', '--workers', '4'] + with pytest.raises(RuntimeError, match='start command runs --workers 4'): + check_rate_limit_topology(app, argv=argv) + + def test_start_command_agreeing_with_the_declaration_is_fine(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='4', + APP_REPLICAS='1', + RATELIMIT_STORAGE_URI='redis://localhost:6379/0', + ) + app = create_app(cfg) + check_rate_limit_topology(app, argv=['/usr/bin/gunicorn', '--workers=4']) + + def test_non_gunicorn_argv_is_ignored(self, fresh_config): + cfg = fresh_config() + app = create_app(cfg) + # pytest's own -w-like flags must not be read as a worker declaration. + check_rate_limit_topology(app, argv=['pytest', '-w', '9']) + + def test_worker_count_parsing(self): + assert worker_count_from_start_command(['gunicorn', '--workers', '3']) == 3 + assert worker_count_from_start_command(['gunicorn', '-w', '2']) == 2 + assert worker_count_from_start_command(['/x/gunicorn', '--workers=7']) == 7 + assert worker_count_from_start_command(['gunicorn', '--bind', ':80']) is None + assert worker_count_from_start_command(['gunicorn', '--workers', 'x']) is None + assert worker_count_from_start_command([]) is None + + +class TestFetchTimeoutHeadroom: + """P9 - API_FETCH_TIMEOUT must stay under gunicorn's --timeout.""" + + def test_fetch_timeout_at_the_worker_budget_raises_in_production(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + API_FETCH_TIMEOUT='60', + ) + app = create_app(cfg) + argv = ['/usr/bin/gunicorn', 'app:create_app()', '--timeout', '60'] + with pytest.raises(RuntimeError, match='API_FETCH_TIMEOUT is 60s'): + check_fetch_timeout_headroom(app, argv=argv) + + def test_fetch_timeout_above_the_worker_budget_raises(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + API_FETCH_TIMEOUT='90', + ) + app = create_app(cfg) + with pytest.raises(RuntimeError, match='--timeout 60s'): + check_fetch_timeout_headroom(app, argv=['gunicorn', '--timeout=60']) + + def test_headroom_is_accepted(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + API_FETCH_TIMEOUT='30', + ) + app = create_app(cfg) + check_fetch_timeout_headroom(app, argv=['gunicorn', '--timeout', '60']) + + def test_outside_production_it_warns_instead_of_raising(self, fresh_config, caplog): + cfg = fresh_config(API_FETCH_TIMEOUT='60') + app = create_app(cfg) + with caplog.at_level(logging.WARNING): + check_fetch_timeout_headroom(app, argv=['gunicorn', '--timeout', '60']) + assert 'API_FETCH_TIMEOUT is 60s' in caplog.text + + def test_non_gunicorn_argv_is_ignored(self, fresh_config): + cfg = fresh_config( + APP_ENV='production', + SECRET_KEY='k', + WEB_CONCURRENCY='1', + APP_REPLICAS='1', + API_FETCH_TIMEOUT='600', + ) + app = create_app(cfg) + # The dev server has no worker to be killed by, and pytest's own flags + # must not be read as a gunicorn timeout. + check_fetch_timeout_headroom(app, argv=['pytest', '--timeout', '5']) + + def test_worker_timeout_parsing(self): + assert worker_timeout_from_start_command(['gunicorn', '--timeout', '45']) == 45 + assert worker_timeout_from_start_command(['gunicorn', '-t', '90']) == 90 + assert worker_timeout_from_start_command(['/x/gunicorn', '--timeout=30']) == 30 + assert worker_timeout_from_start_command(['gunicorn', '--bind', ':80']) is None + assert worker_timeout_from_start_command(['gunicorn', '--timeout', 'x']) is None + assert worker_timeout_from_start_command([]) is None + + +class TestProcessResponseShape: + """3.1-3.3 - the refactor is behavior-preserving and additive only.""" + + EXISTING_KEYS = { + 'success': bool, + 'columns': list, + 'preview': list, + 'total_rows': int, + 'csv_data': list, + 'csv_columns': list, + } + + def _process(self, client, **extra): + data = { + 'input_method': 'paste', + 'pasted_json': json.dumps([{'b': 2, 'a': 1}, {'a': 3, 'c': 4}]), + 'json_path': '(root)', + } + data.update(extra) + return json.loads(client.post('/process', data=data).data) + + def test_no_existing_key_changed_name_type_or_meaning(self, client): + payload = self._process(client) + for key, expected_type in self.EXISTING_KEYS.items(): + assert key in payload, key + assert isinstance(payload[key], expected_type), key + + assert payload['total_rows'] == 2 + assert payload['columns'] == ['a', 'b', 'c'] + assert payload['csv_columns'] == ['a', 'b', 'c'] + assert payload['csv_data'] == [{'b': 2, 'a': 1}, {'a': 3, 'c': 4}] + + def test_preview_limit_is_returned(self, client, app): + app.config['PREVIEW_ROW_LIMIT'] = 7 + payload = self._process(client) + assert payload['preview_limit'] == 7 + + def test_new_keys_are_the_only_additions(self, client): + payload = self._process(client) + added = set(payload) - set(self.EXISTING_KEYS) + assert added == {'preview_limit', 'total_cells', 'max_export_cells'} + + def test_tree_picker_handshake_is_unchanged(self, client): + payload = json.loads( + client.post( + '/process', + data={'input_method': 'paste', 'pasted_json': '{"a": [{"x": 1}]}'}, + ).data + ) + assert payload == {'needs_selection': True, 'raw_json': {'a': [{'x': 1}]}} + + def test_all_error_paths_still_return_their_messages(self, client): + cases = [ + ({'input_method': 'unknown'}, 'Invalid input method'), + ({'input_method': 'paste', 'pasted_json': ''}, 'No JSON provided'), + ({'input_method': 'api', 'api_url': ''}, 'No API URL provided'), + ( + {'input_method': 'paste', 'pasted_json': '{"a": 1}', 'json_path': 'zz'}, + 'not found', + ), + ( + {'input_method': 'paste', 'pasted_json': '{"a": 1}', 'json_path': 'a'}, + 'primitive value', + ), + ] + for data, expected in cases: + response = client.post('/process', data=data) + assert response.status_code == 400, data + assert expected in json.loads(response.data)['error'], data + + def test_process_json_stays_small(self): + """3.1 - the extraction is the point; guard against it creeping back.""" + import ast + import inspect + + import routes + + source = inspect.getsource(routes) + tree = ast.parse(source) + node = next( + n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef) and n.name == 'process_json' + ) + lines = source.splitlines()[node.lineno - 1 : node.end_lineno] + code_lines = [ln for ln in lines if ln.strip() and not ln.strip().startswith('#')] + assert len(code_lines) <= 50, f'process_json is {len(code_lines)} code lines' + + +class TestHealthSplit: + """4.7 - /health/live and /health/ready for load-balancer checks.""" + + def test_original_health_is_unchanged(self, client, app): + data = json.loads(client.get('/health').data) + assert data == {'status': 'ok', 'version': app.config['APP_VERSION']} + + def test_live_reports_the_process_only(self, client, app): + response = client.get('/health/live') + assert response.status_code == 200 + data = json.loads(response.data) + assert data['status'] == 'ok' + assert data['version'] == app.config['APP_VERSION'] + # Liveness must not depend on anything that could fail, or a dependency + # outage turns into a restart loop. + assert 'checks' not in data + + def test_ready_reports_dependencies(self, client): + response = client.get('/health/ready') + assert response.status_code == 200 + data = json.loads(response.data) + assert data['status'] == 'ok' + # No xlsx_writer entry: openpyxl imports at module scope, so a missing + # dependency would stop routes.py loading rather than surface here. + assert data['checks'] == {'rate_limit_storage': 'ok'} + assert data['rate_limit_storage_backend'] == 'memory' + + def test_ready_returns_503_when_a_dependency_is_down(self, client): + with patch('routes.limiter') as mock_limiter: + mock_limiter.storage.check.side_effect = RuntimeError('redis down') + response = client.get('/health/ready') + + assert response.status_code == 503 + data = json.loads(response.data) + assert data['status'] == 'degraded' + assert data['checks']['rate_limit_storage'] == 'unavailable' + # The reason stays in the logs, not the body. + assert 'redis down' not in response.get_data(as_text=True) + + def test_health_endpoints_are_no_store(self, client): + for url in ('/health', '/health/live', '/health/ready'): + assert client.get(url).headers['Cache-Control'] == 'no-store', url + + def test_version_gate_applies_to_all_three(self, fresh_config): + cfg = fresh_config(HEALTH_REVEAL_VERSION='0') + app = create_app(cfg) + for url in ('/health', '/health/live', '/health/ready'): + assert 'version' not in json.loads(app.test_client().get(url).data), url + + +class TestExportDropdownMarkup: + """4.3 / 4.8 - the new formats and the accessibility attributes are present.""" + + def test_new_export_formats_are_offered(self, client): + html = client.get('/').data.decode('utf-8') + for fmt in ('csv', 'tsv', 'jsonl', 'markdown', 'xlsx'): + assert f'data-format="{fmt}"' in html, fmt + + def test_dropdown_is_keyboard_accessible(self, client): + html = client.get('/').data.decode('utf-8') + assert 'aria-haspopup="true"' in html + assert 'aria-expanded="false"' in html + assert 'role="menu"' in html + assert 'role="menuitem"' in html + + def test_about_modal_replaces_alert(self, client, app): + html = client.get('/').data.decode('utf-8') + assert 'id="aboutModal"' in html + # The version comes from config, not a hardcoded string that goes stale. + assert app.config['APP_VERSION'] in html + + def test_no_inline_script_or_style(self, client): + """ + CSP has no unsafe-inline, so an inline handler would simply not run. + + `'