|
| 1 | +# Planned Points — Consolidated Status |
| 2 | + |
| 3 | +**Date:** 2026-09-02 |
| 4 | +**Shipped version:** `Config.APP_VERSION = '1.2.0'` (CHANGELOG entry dated 2026-08-21) |
| 5 | + |
| 6 | +Single index of every point planned across the four planning documents: |
| 7 | + |
| 8 | +- `docs/roadmap-v1.2.md` — Phases 0–5, decisions D1–D6 |
| 9 | +- `docs/security-review-v1.2.md` — findings F1–F17 |
| 10 | +- `docs/performance-review-v1.2.md` — findings P1–P13, perf budget §4 |
| 11 | +- `docs/code-health-final.md` — 2026-05 review: top-10 issues, quick wins, larger refactors |
| 12 | +- `docs/export-budget-v1.2.md` — how `MAX_EXPORT_CELLS` was derived |
| 13 | + |
| 14 | +Status is against the code in this repository, not against the CHANGELOG's own |
| 15 | +claims: every "Done" line below was checked against `config.py`, `routes.py`, |
| 16 | +`helpers.py`, `security.py`, `static/js/app.js`, `Makefile`, `.github/workflows/ci.yml` |
| 17 | +and `requirements*.txt`. |
| 18 | + |
| 19 | +--- |
| 20 | + |
| 21 | +## Section 1 — Done |
| 22 | + |
| 23 | +### 1.1 Security findings (F1–F17) |
| 24 | + |
| 25 | +| ID | Point | Where it landed | |
| 26 | +|---|---|---| |
| 27 | +| F1 | CSV/XLSX formula injection (Critical) | `helpers.sanitize_cell` / `is_formula_trigger` for CSV/TSV/XLSX; XLSX pins `data_type='s'`; JSONL/Markdown deliberately exempt | |
| 28 | +| F2 | 7 dependency CVEs | Flask 3.1.3, requests 2.33.0, gunicorn 22.0.0, openpyxl 3.1.5; pytest moved out of the runtime file into `requirements-dev.txt`; `pip-audit -r requirements.txt` runs in CI | |
| 29 | +| F3 / F9 | Credential leakage into logs; API JSONL `ValueError` → 500 | Fixed log string with no interpolation; JSONL parse failure returns 400 | |
| 30 | +| F4 | User-controlled outbound header name | `routes.is_allowed_outbound_header` — strip + lowercase, then explicit allowlist | |
| 31 | +| F5 | Security-header gaps | HSTS on secure requests, `Permissions-Policy`, COOP, CORP; CSP gained `object-src`, `base-uri`, `frame-ancestors`, `form-action`, `upgrade-insecure-requests` | |
| 32 | +| F6 | SSRF: blocking DNS, no port restriction | Bounded resolver pool with admission control (`API_DNS_*`); `API_ALLOWED_PORTS` default `80,443,8443` | |
| 33 | +| F7 | Default SECRET_KEY accepted in production | `is_production()` fail-fast in the app factory; `env_int()` names the mistyped variable | |
| 34 | +| F8 | Recursion-depth DoS | `extract_table_data` depth cap; `RecursionError` → 400 `JSON nesting too deep` | |
| 35 | +| F10 | 413 returned HTML | JSON handlers for 413/500/404 | |
| 36 | +| F11 | No `Cache-Control: no-store` on data responses | `security.NO_STORE_ENDPOINTS` | |
| 37 | +| F12 | Rate limiting ineffective behind a proxy | Opt-in `TRUST_PROXY=1` → `ProxyFix` with exactly one hop; key derived after ProxyFix | |
| 38 | +| F13 | Upload extension/content-type unchecked | `routes.validate_upload` | |
| 39 | +| F14 | Plain-HTTP credential exposure | Docs-only remediation: HTTPS stated per deploy path; `upgrade-insecure-requests` in CSP | |
| 40 | +| F15 | `/health` version disclosure | Kept on by default, gated by `HEALTH_REVEAL_VERSION` | |
| 41 | +| F16 | Cookie hardening | `HttpOnly`, `SameSite=Lax`, `Secure` tied to `APP_ENV=production` | |
| 42 | +| F17 | Documentation drift | Candidates-handshake references replaced with the tree picker; MIT → GPL-3.0 | |
| 43 | + |
| 44 | +### 1.2 Performance findings (P1–P13) |
| 45 | + |
| 46 | +| ID | Point | Where it landed | |
| 47 | +|---|---|---| |
| 48 | +| P1 | Uncompressed `/process` responses | In-repo gzip middleware (D1), no new dependency; honours `Accept-Encoding` q-values; skips streamed/bodyless/already-encoded responses | |
| 49 | +| P2.2 / P5 | Preview mutation, unbounded nested rendering | `helpers.preview_truncate` returns a capped **copy**; client caps at 20 keys / 20 items / 500 chars | |
| 50 | +| P3 | XLSX fully buffered in memory (High) | Diskless normal-mode workbook + `MAX_EXPORT_CELLS` (measured, default 250 000); CSV/TSV generator-streamed and uncapped; 400 rather than truncation | |
| 51 | +| P4 | Eager tree-picker build | Lazy children on first toggle, per-level and total node caps | |
| 52 | +| P6 | No static cache headers | `STATIC_MAX_AGE` (86 400) with `?v=APP_VERSION` URLs | |
| 53 | +| P7 | Blocking DNS in the request path | Shared bounded resolver pool (shared with F6) | |
| 54 | +| P8 / P12 | Double column scan; multi-copy memory | `flatten_rows` does one pass; API path decodes the `bytearray` without an intermediate copy | |
| 55 | +| P9 | gunicorn timeout vs `API_FETCH_TIMEOUT` | `--timeout 60` everywhere; the invariant is now enforced at startup | |
| 56 | +| P10 | Dead code `find_candidate_arrays` | Removed with its four tests (D2) | |
| 57 | +| P11 | Hardcoded "25" preview badge | `/process` returns `preview_limit` | |
| 58 | +| P13 | Client-side CSV/TSV string building | Chunked Blob builders | |
| 59 | + |
| 60 | +### 1.3 Roadmap phases |
| 61 | + |
| 62 | +- **Phase 0 — Foundations.** Dependency pins, `requirements-dev.txt`, `.github/workflows/ci.yml`, `pyproject.toml` (ruff + pytest), ruff clean, GPL-3.0 correction, `autoDeployTrigger: checksPass`, dead-code removal. |
| 63 | +- **Phase 1 — Security.** All 15 tasks (1.1–1.15); see F-table above. |
| 64 | +- **Phase 2 — Performance.** All 10 tasks (2.1–2.10), including the rate-limit topology guard: `RATELIMIT_STORAGE_URI` is configurable, `WEB_CONCURRENCY` is the single source of truth for workers, `APP_REPLICAS` mirrors `numInstances`, and production refuses to start on `memory://` above one worker × one replica. |
| 65 | +- **Phase 3 — Refactor.** `_load_input` / `_select_table_data` extracted, `process_json` reduced, `openpyxl` imported at module top, `preview_limit` returned, type annotations added to `helpers.py` and `security.py`. |
| 66 | +- **Phase 4 — Features.** 4.1 load more / load all with a 50 000-row DOM guard, 4.2 row filter, 4.3 JSONL + Markdown exports, 4.4 column visibility, 4.5 `#path=` deep links, 4.7 `/health/live` + `/health/ready`, 4.8 in-page About modal and keyboard-accessible export dropdown. (4.6 is not done — see Section 2.) |
| 67 | +- **Phase 5 — Docs/DX.** `MEMORY.md`, `CLAUDE.md`, `AGENTS.md`, `README.md` synced; `.env.example`; `Makefile`; `CHANGELOG.md`; version bumped to 1.2.0. |
| 68 | + |
| 69 | +### 1.4 Decisions closed |
| 70 | + |
| 71 | +| ID | Decision | |
| 72 | +|---|---| |
| 73 | +| D1 | gzip via in-repo middleware, not `Flask-Compress` | |
| 74 | +| D2 | `find_candidate_arrays` deleted in Phase 0, before the depth-guard work | |
| 75 | +| D3 | Proxy trust is opt-in via `TRUST_PROXY`, off by default | |
| 76 | +| D5 | Port allowlist adopted, default `80,443,8443` | |
| 77 | +| D6 | Exports stay diskless and memory-bounded: normal-mode workbook + measured cell budget; `write_only` and `SpooledTemporaryFile` both rejected, each on its own grounds | |
| 78 | + |
| 79 | +### 1.5 Code-health review (2026-05) items now closed |
| 80 | + |
| 81 | +Top-10 issues 1, 2, 3, 4, 5, 7, 8, 9, 10 are done (CI, CVEs, dev/prod dependency |
| 82 | +split, ruff, `process_json` complexity, preview badge, LICENSE, `.env.example`, |
| 83 | +depth guard). Quick wins 1–8 are done. Larger refactors 7.1, 7.3, 7.4 and 7.5 are |
| 84 | +done; 7.2 is partial (see Section 2). |
| 85 | + |
| 86 | +### 1.6 Measurement work |
| 87 | + |
| 88 | +`MAX_EXPORT_CELLS = 250 000` is a measured number, derived by the two-fresh-worker |
| 89 | +`ru_maxrss` protocol recorded in `docs/export-budget-v1.2.md`, with a confirming |
| 90 | +point re-run at the shipped value. |
| 91 | + |
| 92 | +--- |
| 93 | + |
| 94 | +## Section 2 — Pending / Planned |
| 95 | + |
| 96 | +### 2.1 Open decision |
| 97 | + |
| 98 | +- **D4 — opt-in HTTP Basic Auth gate (roadmap 4.6).** Still unresolved; needs |
| 99 | + maintainer sign-off. Scope if approved: `APP_BASIC_AUTH_USER` / `APP_BASIC_AUTH_PASS` |
| 100 | + → `before_request` 401 with constant-time compare and `WWW-Authenticate`, off by |
| 101 | + default, no persistence. Nothing else depends on it. The roadmap's own |
| 102 | + recommendation is to approve. No `BASIC_AUTH` code exists in the repository today. |
| 103 | + |
| 104 | +### 2.2 Known limitation awaiting a decision |
| 105 | + |
| 106 | +- **JSONL export is not a faithful copy of the input document.** Roadmap 4.3 called |
| 107 | + it "lossless — original values". Shipped behaviour: values are written verbatim |
| 108 | + (the security-relevant half), but over the server's *flattened* projection, so |
| 109 | + `{"tags": [1,2]}` exports as `"tags": "[1, 2]"` and `{"meta": {"role": "x"}}` as |
| 110 | + `"meta.role": "x"`. Fixing it means shipping the original rows alongside the |
| 111 | + flattened ones, doubling the payload and client memory that P2/P12 exist to |
| 112 | + reduce. Explicitly flagged for a maintainer decision, not resolved either way. |
| 113 | + |
| 114 | +### 2.3 Accepted exposures, documented rather than fixed |
| 115 | + |
| 116 | +- **Unbounded DNS teardown (F6.1 / P7 residual).** `getaddrinfo` exposes no timeout |
| 117 | + and cannot be cancelled. `API_DNS_TIMEOUT` bounds only the caller's wait; a pool |
| 118 | + thread stays occupied until the platform resolver returns, and interpreter exit |
| 119 | + can still block on it — realistically tens of seconds with glibc defaults. A |
| 120 | + killable subprocess resolver is the documented escalation and remains out of v1.2 |
| 121 | + scope. Best-effort narrowing where the deployment allows: pin |
| 122 | + `options timeout:2 attempts:1` in the container's `resolv.conf`. |
| 123 | +- **DNS rebinding (SSRF residual).** Mitigated by `allow_redirects=False` and |
| 124 | + documented in `routes.py`. Escalation path if the threat model changes from |
| 125 | + internal tool to public service: a custom `HTTPAdapter` that resolves once and |
| 126 | + passes the IP with an explicit `Host` header. |
| 127 | + |
| 128 | +### 2.4 Carried-over engineering work |
| 129 | + |
| 130 | +- **Type annotations (code-health 7.2, roadmap 3.5) — partial.** `helpers.py` and |
| 131 | + `security.py` are annotated. `routes.py` and `config.py` have no annotated |
| 132 | + signatures, and `mypy` is not in `requirements-dev.txt`, `pyproject.toml`, the |
| 133 | + `Makefile`, or CI. The roadmap deliberately marked mypy optional and skipped |
| 134 | + strict mode; the remaining work is the two unannotated modules plus a mypy |
| 135 | + configuration if it is wanted. |
| 136 | +- **Perf budget verification (performance review §4).** The XLSX row of the budget |
| 137 | + was measured. The rest of the table — `/process` transfer size ≤ 2–4 MB, |
| 138 | + `/process` p95 ≤ 3 s, `/process` peak-RSS delta ≤ 50 MiB, tree-picker open |
| 139 | + ≤ 500 ms on a 10 MB payload — is defined but has no recorded run. No perf test |
| 140 | + gates CI (`pytest-benchmark` was deferred by decision), so these stay manual. |
| 141 | +- **Measurement harness is not committed.** The export-budget script is a throwaway; |
| 142 | + re-deriving the budget means rebuilding it from the method section. Committing |
| 143 | + fixtures/generators for the 10 MB reference payload and the 100 k-row `csv_data` |
| 144 | + body would make the budget reproducible rather than re-derivable. |
| 145 | +- **Transitive dependency drift (code-health §12).** Direct dependencies are |
| 146 | + exact-pinned; transitives are not. A lock file (`pip-compile`) was noted as a |
| 147 | + post-review iteration and has not been added. |
| 148 | +- **Docker artifacts.** `README.md` carries Dockerfile and docker-compose templates, |
| 149 | + but no `Dockerfile` is checked in. Phase 5 deliberately kept these docs-only; a |
| 150 | + committed Dockerfile is a fresh decision, not pending roadmap work. |
| 151 | + |
| 152 | +### 2.5 Explicitly out of scope — do not implement without a new decision |
| 153 | + |
| 154 | +These are settled exclusions, listed so they are not mistaken for a backlog: |
| 155 | + |
| 156 | +- Server-side payload caching or session storage for export — violates the |
| 157 | + no-persistence requirement. Perf is served by compression, streaming and |
| 158 | + client-side rendering instead. |
| 159 | +- Redis rate-limit storage as the default. It ships as an optional |
| 160 | + `requirements-redis.txt`, required only above one worker or one replica. |
| 161 | +- Any frontend framework or build step. |
| 162 | +- Any CSP relaxation, `unsafe-inline` included. Phase 1 only tightened it. |
| 163 | +- Payload-level logging or telemetry. |
| 164 | +- Auto keep-alive pings against the Render free tier. |
| 165 | +- Committed `docker-compose` / systemd unit files. |
| 166 | + |
| 167 | +--- |
| 168 | + |
| 169 | +## Verification checklists still requiring a manual sign-off |
| 170 | + |
| 171 | +Two lines in the security review's post-fix checklist are marked manual and have |
| 172 | +no automated equivalent: **F14** (every deployment path states HTTPS is required) |
| 173 | +and **F17** (no stale `candidates` or MIT references remain in the docs). The rest |
| 174 | +of that checklist is covered by `tests/test_routes.py`, `tests/test_security.py` |
| 175 | +and the Node assertions in `tests/js/`. |
0 commit comments