rename privacy "private" → "unlisted"; enable GPS for unlisted
- "unlisted" = not shown in the public feed, but GPS track, timeseries and detail JSON are all accessible by direct URL (security by obscurity) - "private" accepted as legacy alias everywhere (backward compat with existing data on disk) - New writes from Strava sync / ZIP upload / sidecar use "unlisted" - Only "no_gps" now suppresses the GPS track - isUnlisted() helper in format.ts used by all Svelte/Astro components - SCHEMA.md and CLAUDE.md document the privacy model and the distinction between "unlisted" and "no_gps"
This commit is contained in:
Davide Scaini
@@ -0,0 +1,99 @@
|
||||
# Repository Audit — 2026-03-30
|
||||
|
||||
## CRITICAL
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 1 | Security | `edit/server.py:351,386,431,588` | **Path traversal** — `activity_id` from URL is interpolated into filesystem paths without sanitization. Attacker can read/write/delete arbitrary files via `../../` | ✅ Fixed — `_check_id()` validates against `[a-zA-Z0-9\-]+` |
|
||||
| 2 | Security | `edit/server.py:588` | **Path traversal in `delete_image`** — `filename` param is not `.name`-stripped (unlike `upload_image`), enabling deletion of arbitrary files | ✅ Fixed — `Path(filename).name` strip added |
|
||||
| 3 | Security | `edit/server.py:542` | **Path traversal in `upload_activity`** — uploaded `file.filename` used directly for staged path | ✅ Fixed — `Path(file.filename).name` strip added |
|
||||
| 4 | XSS | `ActivityDetail.svelte:67,189` | `marked()` output rendered with `{@html}` without sanitization. Sidecar markdown can inject `<script>` tags | ✅ Fixed — DOMPurify.sanitize() wraps marked() output |
|
||||
|
||||
## HIGH
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 5 | Data | `metrics.py:116-148` | **MMP sliding window on non-contiguous data** — gaps in recording are collapsed, inflating power values | ✅ Fixed — dense 1Hz array with gap zero-fill (standard GoldenCheetah/WKO approach) |
|
||||
| 6 | Data | `metrics.py:170-178` | **Best-effort times on non-contiguous data** — pauses removed from speed array produce artificially fast times | ✅ Fixed — same zero-fill approach, gaps count as 0 km/h |
|
||||
| 7 | Data | `writer.py:85-86` | **Activity ID collision** — two activities with same start second + title silently overwrite each other | ✅ Fixed — collision detected by source_hash comparison; disambiguated with 6-char hash suffix |
|
||||
| 8 | Frontend | `ActivityMap.svelte:88-94` | **Misaligned lat/lon arrays** — independently filtered for nulls, so `lats[0]` may not correspond to `lons[0]`. Start/end markers placed at wrong coordinates | ✅ Fixed — lat/lon filtered as pairs |
|
||||
| 9 | Frontend | `EditDrawer.svelte:121` | **Backdrop dismiss fires `saved` event** with unsaved data, overwriting displayed title/description | ✅ Fixed — backdrop/close now dispatch `close` event |
|
||||
| 10 | Schema | `writer.py` vs `bas-v1.schema.json` | **Writer output fails own schema** — `mmp`, `best_efforts`, `best_climb_m`, `preview_coords`, `custom` written but not declared; schema has `additionalProperties: false` | ✅ Fixed — all missing fields declared in schema |
|
||||
|
||||
## MEDIUM
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 11 | Data | `metrics.py:89-90` | **Falsy check drops `0.0` speed** — `if avg_speed_kmh` treats `0.0` as falsy; should be `is not None` | ✅ Fixed — `is not None` check |
|
||||
| 12 | Data | `parsers/fit.py:89` | Same falsy `0.0` bug for FIT lap `speed_raw` | ✅ Fixed — `is not None` check |
|
||||
| 13 | Data | `metrics.py:222-239` | `_best_climb` filters out `None` elevations, joining non-contiguous segments | ✅ Fixed — `None` samples reset Kadane's window instead of being skipped |
|
||||
| 14 | Parser | `parsers/tcx.py:89-97` | **TCX timestamps with numeric timezone offsets** (`+02:00`) crash — only `Z` suffix handled | ✅ Fixed — numeric offsets parsed with `%z`, converted to UTC |
|
||||
| 15 | Security | `edit/server.py:22` | **CORS `allow_origins=["*"]`** — any website can make requests to the edit server | ✅ Fixed — restricted to `localhost` origins only |
|
||||
| 16 | Security | `edit/server.py:406` | **YAML injection** — `hide_stats` values written into YAML frontmatter without quoting | ✅ Fixed — values filtered against STAT_PANELS allowlist |
|
||||
| 17 | Data | `edit/server.py:497-499` | `save_athlete` mutates immutable `athlete.json` in-place, violating extract immutability | ✅ Fixed — `merge_all()` now applies `edits/athlete.yaml` overlay; server only writes the sidecar |
|
||||
| 18 | Schema | `sport.py` vs schema | **`skiing` sport** produced by code but missing from JSON schema and SCHEMA.md | ✅ Fixed — `skiing` added to schema sport enum |
|
||||
| 19 | Schema | `sport.py` vs schema | **Sub-sport values** `nordic`, `alpine`, `open_water`, `pool` not in schema enum | ✅ Fixed — added to schema sub_sport enum |
|
||||
| 20 | Schema | `SCHEMA.md:140` | **Activity ID format wrong** — docs say `+0200` offset, code uses `Z` UTC | ✅ Fixed — all examples updated to `Z` suffix |
|
||||
| 21 | Frontend | `EditDrawer.svelte:106` | **Regex injection** — filename with metacharacters (e.g. `photo(1).jpg`) breaks `deleteImage` regex | ✅ Fixed — filename escaped before RegExp construction |
|
||||
| 22 | Frontend | `EditDrawer.svelte:85-99` | No try/catch in `uploadImages` — network error leaves UI stuck in uploading state | ✅ Fixed — try/catch with `finally { uploading = false }` |
|
||||
| 23 | Frontend | `RecordsView.svelte:99`, `Base.astro:220` | Hardcoded `/activity/` URLs ignore `BASE_URL`, breaking subdirectory deployments | ✅ Fixed — `base` prop threaded through `AthleteView` → `RecordsView`; `Base.astro` uses `baseUrl` from `import.meta.env.BASE_URL` |
|
||||
|
||||
## LOW
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 24 | Data | `cli.py:341-350` | `_patch_duplicate_of` silently swallows all exceptions | ✅ Fixed — logs warning via `logging.warning` instead of bare `pass` |
|
||||
| 25 | Data | `timeseries.py:26-35` | Non-monotonic `t` array if input points aren't sorted | ✅ Fixed — `t <= last_t` guard rejects both duplicate and backwards timestamps |
|
||||
| 26 | Data | `simplify.py:49-50` | `preview_coords` returns `max_points + 1` elements (off-by-one) | ✅ Fixed — subsample to `max_points - 1` then append last point |
|
||||
| 27 | Config | `render/cli.py:42`, `edit/cli.py:66` | `yaml.safe_load` returns `None` for empty config → `AttributeError` | ✅ Fixed — `or {}` fallback on both load sites |
|
||||
| 28 | Frontend | `format.ts:18` | `formatDuration` doesn't floor seconds — fractional values display messily | ✅ Fixed — `Math.floor(s)` before any arithmetic |
|
||||
| 29 | Frontend | `StatsView.svelte:31` | No error handling on `index.json` fetch | ✅ Fixed — try/catch with error display |
|
||||
| 30 | Frontend | `ActivityMap.svelte` | No `map.resize()` on container size change | ✅ Fixed — `ResizeObserver` calls `map.resize()` |
|
||||
| 31 | Test | `test_writer.py:21-30` | Weak assertions — only check substrings, would pass with malformed IDs | ✅ Fixed — exact equality assertions on full ID strings |
|
||||
| 32 | Test | `test_sport.py` | Zero test coverage for `normalise_sub_sport` | ✅ Fixed — 3 new tests covering Strava CamelCase, ski variants, and unknown→None |
|
||||
| 33 | Test | `test_merge.py:70` | Test uses `sport: "gravel"` which isn't a valid BAS sport | ✅ Fixed — changed to `"running"` |
|
||||
|
||||
---
|
||||
|
||||
## Second-pass audit — 2026-04-01
|
||||
|
||||
### CRITICAL / HIGH (new)
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 34 | Data | `writer.py:88-93` | **Disambiguated ID not written into JSON body** — collision adds hash suffix to filename but `detail["id"]` still holds original ID | ✅ Fixed — `detail["id"] = activity_id` after disambiguation |
|
||||
| 35 | Data | `cli.py:79-85` | **`write_activity` return value ignored** — caller uses pre-collision ID for `build_summary`, so index links are broken for disambiguated activities | ✅ Fixed — `activity_id = write_activity(...)` captures the canonical ID |
|
||||
| 36 | Data | `writer.py:88-92` | **TOCTOU race in collision guard** — two workers processing same-timestamp activities can both see no existing file and overwrite each other | ✅ Fixed — hybrid pending-file approach: workers write to unique `.pending.json` files, main process arbitrates by quality score and does atomic rename via `finalize_pending()` |
|
||||
| 37 | Schema | `bas-v1.schema.json:90-128` | **`activity_summary` missing `custom` property** — `merge.py` always adds `custom` but schema has `additionalProperties: false` | ✅ Fixed — `custom` added to `activity_summary` |
|
||||
| 38 | Frontend | `EditDrawer.svelte:101` | **Undeclared `error` variable** in uploadImages catch — runtime `ReferenceError` crash on upload failure | ✅ Fixed — uses `saveStatus` with `saveOk = false` |
|
||||
|
||||
### MEDIUM (new)
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 39 | Security | `edit/server.py:409` | **Sport value not validated/quoted** before YAML write — YAML injection possible | ✅ Fixed — validated against `SPORTS` allowlist |
|
||||
| 40 | Security | `edit/server.py:446` | **No image content-type validation** — arbitrary files accepted and served as images; HTML upload executes JS in site origin | ✅ Fixed — rejects non-`image/*` content types |
|
||||
| 41 | Security | `edit/server.py:270-274` | **XSS via unescaped filename in `innerHTML`** — `renderImageList` interpolates filenames into HTML without escaping | ✅ Fixed — `escapeHtml()` applied to filenames |
|
||||
| 42 | Frontend | `MmpChart.svelte:154` | **ResizeObserver captures stale closure** — after toggling range selections, resize renders old data | ✅ Fixed — reactive variables keep closure current |
|
||||
| 43 | Schema | `SCHEMA.md:113` | **Sport enum missing "skiing"** — JSON schema has it, SCHEMA.md doesn't | ✅ Fixed — `skiing` added, sub_sport examples updated |
|
||||
| 44 | Schema | `SCHEMA.md:109-129` | **Summary fields table incomplete** — missing `sub_sport`, `mmp`, `best_efforts`, `best_climb_m`, `preview_coords` | ✅ Fixed — all fields added to summary table |
|
||||
| 45 | Config | `config.py:61` | **Empty config file still crashes** — `yaml.safe_load` returns `None`, no `or {}` fallback (round 1 fix applied to render/edit CLIs but not here) | ✅ Fixed — `or {}` fallback added |
|
||||
| 46 | Frontend | `Base.astro:101-106` | **Nav links ignore `BASE_URL`** — hardcoded `/`, `/stats/`, `/athlete/` break subpath deployments | ✅ Fixed — nav uses `baseUrl` variable |
|
||||
| 47 | Data | `render/merge.py:161` | **`athlete.yaml` merge has no field allowlist** — server restricts editable fields, but merge applies all keys | ✅ Fixed — merge uses `_ATHLETE_EDITABLE` allowlist matching server |
|
||||
| 48 | Frontend | `types.ts:102` | **`ActivityDetail extends ActivitySummary`** inherits `detail_url`, `track_url`, `preview_coords` which detail objects never have | ✅ Fixed — `Omit<>` excludes summary-only fields |
|
||||
|
||||
### LOW (new)
|
||||
|
||||
| # | Area | File | Issue | Status |
|
||||
|---|------|------|-------|--------|
|
||||
| 49 | Parser | `parsers/tcx.py:101` | **Timezone offsets without colon** (`+0200`) not handled — only `+02:00` form matched | ✅ Fixed — regex accepts optional colon (`[+-]\d{2}:?\d{2}`) |
|
||||
| 50 | Parser | `parsers/gpx.py:59-78` | **Power data in GPX extensions not parsed** — MMP always None for GPX files with power meters | ✅ Fixed — parses `pwr`, `power`, `watts` extension tags |
|
||||
| 51 | Data | `simplify.py:63-71` | **`speeds` array not filtered same as `coordinates`** — could misalign if `simplify_track` changes | ✅ Fixed — speeds array uses same lat/lon null filter |
|
||||
| 52 | Frontend | `ActivityCharts.svelte` | **No `onDestroy` cleanup** — chart SVG not removed on unmount, minor memory leak | ✅ Fixed — `onDestroy` removes chart SVG |
|
||||
| 53 | Frontend | `ActivityCharts.svelte:83-86` | **`resetTrim` guard always true** — trim doesn't reset on tab change if min/max happen to match | ✅ Fixed — tracks `lastResetTab` to force reset on tab switch |
|
||||
| 54 | Frontend | `AthleteView.svelte:30` | **No validation of URL `tab` parameter** — invalid value shows blank content | ✅ Fixed — validates against `TABS` array, falls back to `'power'` |
|
||||
| 55 | Test | `test_writer.py` | **No tests for `write_activity`, `build_summary`, `write_index`, `write_athlete_json`** | ✅ Partial — added `test_build_summary_required_fields` and `test_id_utc_conversion` |
|
||||
| 56 | Test | `test_sport.py` | **No test for `normalise_sport("skiing")` or swimming variants** | ✅ Fixed — added `test_skiing_variants` and `test_swimming_variants` |
|
||||
| 57 | Test | `test_merge.py:58,122` | **Non-canonical IDs in test fixtures** — use colons and underscores, don't match real ID format | ✅ Fixed — IDs use canonical `2024-01-01T080000Z-morning-ride` format |
|
||||
| 58 | Data | `edit/server.py:549-551` | **No upload size limit** — file written to disk before validation | ✅ Fixed — 50 MB limit enforced before writing to disk |
|
||||
| 59 | Data | `edit/server.py:586` | **Exception message leaks internal paths** — `str(exc)` returned to client in 422 response | ✅ Fixed — returns `type(exc).__name__` only, no internal details |
|
||||
Reference in New Issue
Block a user