|
1 | 1 | # Audit Issues |
2 | 2 |
|
3 | | -- Date: 2026-04-02 |
4 | | -- Commit: `2ad80ce49f12` |
5 | | -- Codebase summary (`sloc`): 49 files, 2,470 code LOC, 1,290 documentation LOC, 5,670 total lines; main languages: Python 1,654 LOC, YAML 353 LOC, Rust 249 LOC. |
6 | | -- Scope: Python wrapper (`mlnative/`), Rust renderer (`rust/`), example servers (`examples/`), CI/release workflows (`.github/workflows/`), and helper scripts (`scripts/`). |
| 3 | +- Date: 2026-04-03 |
| 4 | +- Commit: `9bd2eac59da5` |
| 5 | +- Codebase summary (`sloc`): 50 files, 2,696 code LOC, 1,271 documentation LOC, 6,004 total lines; main languages: Python 1,869 LOC, YAML 353 LOC, Rust 259 LOC. |
| 6 | +- Scope: Python wrapper (`mlnative/`), Rust renderer (`rust/`), example servers (`examples/`), CI/release workflows (`.github/workflows/`), packaging/build helpers, and test/runtime container files. |
7 | 7 | - Audit references: [OWASP ASVS 5.0](https://owasp.org/www-project-application-security-verification-standard/) with emphasis on [V15 Secure Coding and Architecture](https://raw.githubusercontent.com/OWASP/ASVS/v5.0.0/5.0/en/0x24-V15-Secure-Coding-and-Architecture.md), [V16 Security Logging and Error Handling](https://raw.githubusercontent.com/OWASP/ASVS/v5.0.0/5.0/en/0x25-V16-Security-Logging-and-Error-Handling.md), and [grugbrain.dev](https://grugbrain.dev/). |
| 8 | +- Local dependency lookup: Renovate dry-run succeeded on 2026-04-03; the only actionable delta surfaced was `actions/attest-build-provenance` digest `b3e506e8c389afc651c5bacf2b8f2a1ea0557215` → `a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32`. |
8 | 9 |
|
9 | 10 | ## Prioritized findings |
10 | 11 |
|
11 | | -### 1) User-controlled `style` reaches network and filesystem loaders |
12 | | -- Location: `examples/fastapi_server.py:42-76`, `examples/web_test_server.py:43-126`, `mlnative/map.py:118-136`, `rust/src/main.rs:82-97` |
| 12 | +### 1) Reused temp style file is truncated but not rewound before rewrite |
| 13 | +- Location: `rust/src/main.rs:89-99`, `tests/test_issue_resolutions.py:138-145` |
13 | 14 | - Category: security |
14 | | -- Reference: ASVS `v5.0.0-15.3.2`, `v5.0.0-15.2.5` |
15 | | -- Recommendation: Do not accept arbitrary style URLs/paths from request data. Expose style IDs from an allowlist, reject local paths/`file://` in server contexts, and keep redirect behavior explicit. |
16 | | -- Status: open |
17 | | - |
18 | | -### 2) Example render endpoints are expensive, unauthenticated, and bind publicly by default |
19 | | -- Location: `examples/fastapi_server.py:33-101`, `examples/web_test_server.py:96-155` |
20 | | -- Category: security |
21 | | -- Reference: ASVS `v5.0.0-15.1.3`, `v5.0.0-15.2.2` |
22 | | -- Recommendation: Keep demo servers on `127.0.0.1` by default and add auth, rate limits, concurrency caps, request budgets, and caching before any non-local exposure. |
23 | | -- Status: open |
24 | | - |
25 | | -### 3) Errors are leaked to clients while renderer stderr is discarded |
26 | | -- Location: `examples/fastapi_server.py:82-83`, `examples/web_test_server.py:132-140`, `mlnative/_bridge.py:114-120` |
27 | | -- Category: security |
28 | | -- Reference: ASVS `v5.0.0-16.5.1`, `v5.0.0-16.3.4` |
29 | | -- Recommendation: Return generic client errors, log structured exceptions server-side, and retain renderer stderr behind a debug/logging path instead of dropping it. |
| 15 | +- Reference: ASVS `v5.0.0-16.5.3` |
| 16 | +- Recommendation: Seek to offset 0 before rewriting the reused `NamedTempFile` (or recreate it), then add a regression test that reloads a JSON style twice. As written, repeat `reload_style()` / `set_geojson()` calls can produce a NUL-prefixed style file and break rendering. |
30 | 17 | - Status: open |
31 | 18 |
|
32 | | -### 4) Native binary resolution still trusts the first `PATH` hit |
33 | | -- Location: `mlnative/_bridge.py:70-92` |
| 19 | +### 2) Renderer timeouts can desynchronize the daemon protocol and hand late responses to the wrong caller |
| 20 | +- Location: `mlnative/_bridge.py:181-225`, `mlnative/_bridge.py:277-306` |
34 | 21 | - Category: security |
35 | | -- Reference: ASVS `v5.0.0-15.2.4`, `v5.0.0-15.2.5` |
36 | | -- Recommendation: Prefer packaged binaries only by default. If overrides are needed, gate them behind explicit opt-in and verify ownership/hash/signature. |
| 22 | +- Reference: ASVS `v5.0.0-15.4.1`, `v5.0.0-16.5.3` |
| 23 | +- Recommendation: Treat a timeout as terminal for that daemon instance, or add request IDs and response matching. Today a timed-out command leaves its eventual response in the shared queue, so the next command can consume stale data. |
37 | 24 | - Status: open |
38 | 25 |
|
39 | | -### 5) Binary download helper executes unverified release artifacts |
40 | | -- Location: `scripts/download-binary.py:36-74` |
| 26 | +### 3) Example servers still let request data choose arbitrary remote URLs and local style files |
| 27 | +- Location: `examples/fastapi_server.py:42-75`, `examples/web_test_server.py:44-129`, `examples/templates/test_form.html:61-63`, `mlnative/map.py:122-139`, `rust/src/main.rs:82-102` |
41 | 28 | - Category: security |
42 | | -- Reference: ASVS `v5.0.0-15.2.4` |
43 | | -- Recommendation: Verify checksums/signatures before `chmod +x`, or remove the helper and rely on trusted package distribution only. |
| 29 | +- Reference: ASVS `v5.0.0-15.3.2`, `v5.0.0-15.2.5` |
| 30 | +- Recommendation: In server contexts, expose style IDs from an allowlist instead of raw `style` strings, reject local paths / `file://`, and run the renderer with explicit egress restrictions if any untrusted input remains. |
44 | 31 | - Status: open |
45 | 32 |
|
46 | | -### 6) CI dependency scanning is currently broken; maintenance updates are also pending |
47 | | -- Location: `.github/workflows/ci.yml:77-91`, `pyproject.toml:23-40`, `.mise.toml:1-5`, `rust/Cargo.toml:9-15` |
| 33 | +### 4) Render endpoints are expensive but ship without auth, quotas, caching, or concurrency budgets |
| 34 | +- Location: `examples/fastapi_server.py:41-79`, `examples/web_test_server.py:112-129`, `examples/production_deployment.py:38-67` |
48 | 35 | - Category: security |
49 | | -- Reference: ASVS `v5.0.0-15.1.1`, `v5.0.0-15.2.1` |
50 | | -- Recommendation: Make the scan runnable in CI (`uvx pip-audit`, or add `pip-audit` to the CI environment) and review the local Renovate lookup deltas, notably `tempfile 3.23→3.27`, `rust 1.90.0→1.94.1`, `just 1.46.0→1.48.1`, `python 3.12→3.14.3`, and the `actions/attest-build-provenance` digest refresh. |
| 36 | +- Reference: ASVS `v5.0.0-15.1.3`, `v5.0.0-15.2.2` |
| 37 | +- Recommendation: Before any non-local exposure, add authentication, per-client rate limits, worker/concurrency caps, cache hot responses, and explicit time/size budgets. |
51 | 38 | - Status: open |
52 | 39 |
|
53 | | -### 7) `fit_bounds()` accepts `-90` latitude then falls into a raw `math domain error` |
54 | | -- Location: `mlnative/map.py:317-344` |
55 | | -- Category: security |
56 | | -- Reference: ASVS `v5.0.0-16.5.3` |
57 | | -- Recommendation: Reject/clamp to Web Mercator-safe latitude bounds before projection and convert failures to `MlnativeError` consistently. |
| 40 | +### 5) The “production-ready” pool example can block forever and its health check performs a full remote render |
| 41 | +- Location: `examples/production_deployment.py:58-67`, `examples/production_deployment.py:99-104` |
| 42 | +- Category: performance |
| 43 | +- Reference: ASVS `v5.0.0-15.1.3`; grugbrain.dev |
| 44 | +- Recommendation: Use bounded wait times on `Queue.get()`, return overload errors instead of hanging, and split cheap liveness checks from heavyweight render/dependency checks. |
58 | 45 | - Status: open |
59 | 46 |
|
60 | | -### 8) Batch rendering has no request cap and buffers all PNGs in memory at once |
61 | | -- Location: `mlnative/map.py:229-278`, `mlnative/_bridge.py:166-176`, `rust/src/main.rs:287-327` |
| 47 | +### 6) Batch rendering still buffers the full response set in memory twice |
| 48 | +- Location: `rust/src/main.rs:301-337`, `mlnative/_bridge.py:197-210`, `mlnative/_bridge.py:338-364`, `mlnative/map.py:238-246` |
62 | 49 | - Category: performance |
63 | 50 | - Reference: ASVS `v5.0.0-15.1.3`, `v5.0.0-15.2.2` |
64 | | -- Recommendation: Add view-count/output-size limits and prefer streaming or chunked framing over whole-batch buffering for large jobs. |
| 51 | +- Recommendation: Keep the view/pixel caps, but move to per-image streaming or chunked framing. The Rust side accumulates every PNG before send, and the Python side then reads the combined payload and copies each slice again. |
65 | 52 | - Status: open |
66 | 53 |
|
67 | | -### 9) JSON style reloads leak temp files for the daemon lifetime |
68 | | -- Location: `rust/src/main.rs:65-94`, `rust/src/main.rs:140-146` |
69 | | -- Category: performance |
70 | | -- Reference: ASVS `v5.0.0-15.1.3` |
71 | | -- Recommendation: Reuse a single temp style file or drop old `NamedTempFile`s before pushing new ones; repeated `reload_style()`/`set_geojson()` currently grows file descriptors and temp storage monotonically. |
| 54 | +### 7) `set_geojson()` still reloads the entire style document for every source update |
| 55 | +- Location: `mlnative/map.py:438-482` |
| 56 | +- Category: complexity |
| 57 | +- Reference: ASVS `v5.0.0-15.1.3`; grugbrain.dev (“complexity very, very bad”) |
| 58 | +- Recommendation: Add source-level mutation in Rust or separate immutable style from mutable data. The current full-style rewrite is a simple API, but it makes frequent data updates expensive and harder to reason about. |
72 | 59 | - Status: open |
73 | 60 |
|
74 | | -### 10) `set_geojson()` reloads the entire style for each source update |
75 | | -- Location: `mlnative/map.py:413-451` |
| 61 | +### 8) Release builds create “platform wheels” by renaming a pure `py3-none-any` wheel |
| 62 | +- Location: `Justfile:96-114`, `.github/workflows/release.yml:83-90`, `pyproject.toml:54-57` |
76 | 63 | - Category: complexity |
77 | | -- Reference: ASVS `v5.0.0-15.1.3`; grugbrain.dev (“complexity very, very bad”) |
78 | | -- Recommendation: Add source-level mutation in Rust, or clearly document/cap the full-style reload cost for update-heavy workloads. |
| 64 | +- Reference: ASVS `v5.0.0-15.1.2`; grugbrain.dev |
| 65 | +- Recommendation: Use a real platform-wheel pipeline (`cibuildwheel`, `auditwheel`, `delocate`, `maturin`, or equivalent) or rewrite embedded wheel metadata consistently. The current release job renames the filename only; a local build still emits `Root-Is-Purelib: true` and `Tag: py3-none-any`. |
79 | 66 | - Status: open |
80 | 67 |
|
81 | | -### 11) `geopy` is a required runtime dependency even though the library core does not use it |
82 | | -- Location: `pyproject.toml:24-29`, `README.md:21-25`, `examples/address_rendering.py:1-26` |
| 68 | +### 9) CI integration tests depend on live third-party network and remote tile/style services |
| 69 | +- Location: `tests/test_render.py:34-39`, `tests/test_render.py:85-95`, `.github/workflows/ci.yml:138-222`, `docs/CI.md:20-33`, `docs/CI.md:128-134` |
83 | 70 | - Category: complexity |
84 | | -- Reference: ASVS `v5.0.0-15.2.3`; grugbrain.dev |
85 | | -- Recommendation: Move geocoding dependencies into an example/extra (for example `geo` or `examples`) so the core renderer ships with a smaller attack and maintenance surface. |
| 71 | +- Reference: ASVS `v5.0.0-15.1.3`; grugbrain.dev |
| 72 | +- Recommendation: Move CI to local fixtures, a pinned test style, or a local mock tile server. Current smoke/integration coverage is useful, but it is also coupled to OpenFreeMap availability and network latency. |
| 73 | +- Status: open |
| 74 | + |
| 75 | +### 10) Test container still bootstraps toolchain code via `curl | sh` |
| 76 | +- Location: `Dockerfile.test:21` |
| 77 | +- Category: security |
| 78 | +- Reference: ASVS `v5.0.0-15.2.4` |
| 79 | +- Recommendation: Pin a checksum or versioned installer artifact, or use a trusted package source instead of piping a live script directly into `sh`. |
| 80 | +- Status: open |
| 81 | + |
| 82 | +### 11) Release provenance action digest is already stale per local Renovate lookup |
| 83 | +- Location: `.github/workflows/release.yml:161-163`, `.github/workflows/release.yml:200-202`, `renovate.json:1-3` |
| 84 | +- Category: security |
| 85 | +- Reference: ASVS `v5.0.0-15.1.1`, `v5.0.0-15.2.1` |
| 86 | +- Recommendation: Refresh `actions/attest-build-provenance` to digest `a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32` and keep digest refreshes within the repo’s documented remediation window. |
86 | 87 | - Status: open |
87 | 88 |
|
88 | 89 | ## Resolved log |
89 | | -- Previously flagged workflow action drift is resolved in the current tree: committed workflow YAML is digest-pinned (`.github/workflows/ci.yml`, `.github/workflows/release.yml`). |
90 | | -- Previously flagged predictable temp style filenames are resolved: the Rust daemon now uses `tempfile::NamedTempFile` (`rust/src/main.rs`). |
91 | | -- Previously flagged unit-test selection drift is resolved: `just test-unit` now excludes only `integration` tests (`Justfile:34-36`). |
92 | | -- Previously flagged floating tool versions are resolved: `.mise.toml` now pins Python, Rust, Just, and uv (`.mise.toml:1-5`). |
93 | | -- Previously flagged dead vendored Node renderer surface is resolved in the current tree: the tracked JS renderer/vendor files are gone from `mlnative/`. |
94 | | -- Previously flagged Rust patch lag is resolved: `maplibre_native` and `image` are already at the looked-up current versions in `rust/Cargo.toml`. |
95 | | -- Previously flagged PNG base64 transport/thread-per-command issues are resolved: the current bridge uses a persistent reader thread and raw binary payload framing (`mlnative/_bridge.py`, `rust/src/main.rs`). |
| 90 | +- Example servers now bind to loopback by default instead of `0.0.0.0` (`examples/fastapi_server.py:112`, `examples/web_test_server.py:165`). |
| 91 | +- Native binary lookup now prefers packaged binaries and requires explicit opt-in for `PATH` fallback (`mlnative/_bridge.py:101-110`, `tests/test_bridge.py:33-57`). |
| 92 | +- The old binary download helper now fails closed instead of downloading and executing an unverified artifact (`scripts/download-binary.py:36-53`). |
| 93 | +- CI security scanning is wired back in via `uv run --frozen --with pip-audit pip-audit` (`.github/workflows/ci.yml`). |
| 94 | +- `fit_bounds()` now enforces Web Mercator-safe latitude bounds and fails with `MlnativeError` instead of raw projection errors (`mlnative/map.py:333-352`). |
| 95 | +- `geopy` is no longer a core runtime dependency; it lives in the optional `geo` extra (`pyproject.toml:26-33`). |
0 commit comments