|
1 | 1 | # Decision Log |
2 | 2 |
|
3 | | -> **Status: stub file** |
4 | | -> |
5 | | -> This file is reserved by Phase 0 task 0.5 of [docs/PUBLIC_RELEASE_PLAN.md](PUBLIC_RELEASE_PLAN.md). |
6 | | -> |
7 | | -> Content will be migrated from the "Decision Log" section of `CLAUDE.md` |
8 | | -> when Phase 0.5 executes. Until then, the authoritative Decision Log |
9 | | -> lives in the gitignored `CLAUDE.md`. |
| 3 | +Architectural and tech-stack decisions made across the project's lifetime. Each row captures: the decision, alternatives considered, why this option won, and the reversal cost. |
| 4 | + |
| 5 | +This file is the canonical public-facing source. A mirror copy lives in the gitignored CLAUDE.md as a personal-overlay shortcut — but THIS file is authoritative. When a new decision lands, it goes here first. |
| 6 | + |
| 7 | +Plan-reference: Phase 0.5 migration (PUBLIC_RELEASE_PLAN.md, DECISION_LOG D11). |
| 8 | + |
| 9 | +--- |
| 10 | + |
| 11 | + |
| 12 | +When making a tech choice with multiple valid options, record it here BEFORE deploy. Skip trivial choices (which `for` loop, variable name). Record only choices where ALL of these are true: |
| 13 | + |
| 14 | +- Multiple valid options exist |
| 15 | +- The losing option could plausibly be re-proposed in 6 months by someone (or a future Claude session) who did not see this discussion |
| 16 | +- Reverting later would require non-trivial work (data migration, key rotation, schema change, user retraining) |
| 17 | + |
| 18 | +**Format** (one row per decision; append, never edit historical rows): |
| 19 | + |
| 20 | +| Date | Decision | Alternatives considered | Why this won | Reversal cost | |
| 21 | +|------|----------|-------------------------|--------------|---------------| |
| 22 | +| 2026-04-12 | HMAC key derived from Splunk GUID at runtime (1h cache TTL) | Static key in app.conf; PBKDF2 from passphrase | Auto-rotates on container rebuild; no key in source; no manual rotation | Medium — every GUID change requires `reset_cooldowns.sh` + FIM rebuild | |
| 23 | +| 2026-04-12 | Dual-store FIM baseline (file + KV collection) | File-only baseline | Attacker can not silently re-baseline by editing the file alone | Low — delete both stores, FIM auto-rebuilds in 60s | |
| 24 | +| 2026-04-13 | KV-store cooldowns instead of JSON file | JSON file with HMAC + flock | Survives concurrent writes without file locking; queryable from SPL | Medium — requires `wl_migrate_cooldowns.py` for schema changes | |
| 25 | +| 2026-04-14 | FIM polling 60s → 15s + 2s stat-watcher | Keep at 60s; switch to inotify | Benchmarked at 0.6% CPU; inotify needs Linux-only deps Splunk container lacks | Low — change `inputs.conf` interval back to 60 | |
| 26 | +| 2026-04-15 | Lockdown-exempt deploy windows | All actions blocked during lockdown | Allows hotfix deploys during incident; sentinel mutations stay HIGH | Low — remove from `LOCKDOWN_EXEMPT_ACTIONS` set | |
| 27 | +| 2026-04-19 | Scheduled expiration cleanup compares legacy (no-suffix) rows against UTC (`tz_offset_minutes=0`) | Query user's tz from Splunk prefs per row; keep prior server-local-time behavior | Server-side scheduled job has no browser tz context; UTC is deterministic across Splunk hosts and matches the handler's contract. Analysts needing precise tz should use the new " UTC"-suffixed format. | Low — change `SCHEDULED_TZ_OFFSET_MINUTES` constant in `bin/wl_expiration_cleanup.py` | |
| 28 | +| 2026-04-21 | `wl_fim_watch.py` emits only state-change events (`fim_watch_started`/`fim_watch_stopped`); no periodic heartbeat | Keep the 5-minute heartbeat (288 events/day); increase interval to 1h (24/day); route heartbeat to `index=_internal` | At 288 events/day × N instances the heartbeat dominates `index=wl_audit` storage and floods the "File Integrity Monitor Alerts" panel, burying real alerts. `splunkd` supervises the watcher via `interval=0`, so a dead process is auto-restarted and each restart fires a fresh `fim_watch_started` — a flapping watcher surfaces as repeated starts without matching stops. **Known trade-off**: a watcher that hangs without exiting (no SIGKILL, no exit) is undetectable from the audit trail alone. | Low — reintroduce a `HEARTBEAT_INTERVAL` constant + emit block in `bin/wl_fim_watch.py` around line 640; dashboard filter in `audit.xml` is belt-and-suspenders and harmless if left in place | |
| 29 | +| 2026-04-22 | RequireJS `urlArgs: "_b=<build>"` cache-bust in `appserver/static/whitelist_manager.js` kept in sync with `app.conf` `build` | Document "hard-refresh after every deploy" in README; rename JS files on every deploy; force Splunk to regenerate `@<server-hash>` (not user-controllable) | Splunk sends `Cache-Control: public, max-age=31536000` on `/static/@<server-hash>/...` assets. The `@<hash>` is the Splunk server-build identifier — bumping `app.conf build` does NOT change it, so bumped app versions don't invalidate browser cache. Users kept running stale JS for days; build-607 content_hash fix was invisible to them until I diagnosed cache layer. urlArgs appends `?_b=<N>` to every AMD module URL, forcing a fresh fetch each build bump. | Low — remove the `require.config({ urlArgs: ... })` line from `whitelist_manager.js`; reverts to stale-cache behavior. **Maintenance rule**: bumping `build` in `app.conf` REQUIRES also bumping `_b=` in `whitelist_manager.js` (same number) — otherwise new JS will not reach users. | |
| 30 | +| 2026-04-23 | Mechanical PreToolUse hook (`.claude/hooks/block-synthetic-fixtures.js`) blocks direct writes to Splunk-internal state during feature verification; paired with prose rule in CLAUDE.md ("Synthetic Fixtures — Banned for Feature Verification") | Prose-only rule in CLAUDE.md (same as before the incident); rely on self-discipline; per-commit review checklist | Prose-only was the existing state, and it failed to prevent the build 614 "Invalid Date" incident (synthetic `_approval_queue.json` injection hid the dual-admin `timestamp` vs `submitted_at` schema drift). Hooks cannot be rationalized away the way prose rules can; Claude reads CLAUDE.md once per session but the hook runs every `Write`/`Edit`/`Bash`. Path-gated exception for `tests/unit/**` preserves pure-helper unit testing; `# JUSTIFIED: <reason>` marker on Bash covers the first-install-bootstrap edge case. | Low — delete the PreToolUse entry from `.claude/settings.json`. `.claude/` is git-ignored so the hook is per-developer; if reversed, no shared state changes. Pairs with `feedback_synthetic_fixtures_mask_schema_drift.md` for the lesson. | |
| 31 | +| 2026-04-26 | Strict-ASCII policy on detection rule names, CSV filenames, approval reasons, comments, and `app_context`; enforced at BOTH outer wrapper (`_submit_create_delete_approval`) AND inner choke point (`_submit_approval`) plus pipeline replay; null bytes + control chars + zero-width + bidi-override + fullwidth + combining marks all rejected. | Frontend-only ASCII regex (existing `wl_modals.js` pre-flight check); Python `c.isalnum()` fallback (Unicode-aware); accept Unicode but normalize via NFC/NFKC before storage | Frontend-only is bypassable with a 30-second curl call; `c.isalnum()` is Unicode-aware so it accepted CJK/Cyrillic/Greek "letters" and made the validator a no-op for the attacks we cared about. Strict ASCII (regex `^[A-Za-z0-9_\-. ]+$`) closes the homoglyph + filesystem-path + SPL-identifier attack surface in one rule. The dual-gate placement was discovered live: the outer wrapper alone left a bypass via direct `action=submit_approval` POST, which my own sanity-test exposed in this session. NFC/NFKC normalization rejected because (a) it doesn't help against bidi/zero-width attacks, (b) it adds a stateful step before validation that an attacker can probe for timing oracles, (c) ASCII is the operational reality — every dashboard, audit search, and rule_csv_map.csv export consumes ASCII anyway. | Low — revert `is_ascii_name`/`is_valid_app_context` callers to `c.isalnum()`-style checks; queue entries with the old policy continue to work because `_execute_replay_create_csv` has an explicit `path is None` guard that fails cleanly. **Maintenance rule**: when adding a new payload field that flows into a filesystem path, SPL search, audit log, or display string, run `validate_ascii_text` on it at the gate AND add a unit test in `tests/unit/test_ascii_validation.py` pinning the rejection. | |
| 32 | +| 2026-05-01 | **Kill the `wl-btn` / `wl-btn-primary` / `wl-btn-danger` taxonomy entirely** — migrate every site to Splunk-bundled `btn` / `btn-primary` / `btn-danger` (build 631-632). Preserve only `wl-btn-locked` (the opacity helper used by approval-lock UX in `wl_table.js` and `wl_approval_ui.js`); add proper CSS rules for `btn-success` / `btn-danger` / `btn-warning` which Splunk's bundle ships unstyled. | (a) Keep both taxonomies and add CSS rules for `wl-btn` / `wl-btn-primary` / `wl-btn-danger`; (b) keep `wl-btn` as the canonical app system and provide rules for it; (c) migrate fully to Splunk-bundled and remove `wl-btn` from production. | The 2026-04-30 UI consistency audit found that `wl-btn`, `wl-btn-primary`, and `wl-btn-danger` had ZERO matching rules in `whitelist_manager.css` — only `.wl-btn-locked` (opacity helper) existed. Sites using these classes rendered as plain text (transparent bg, 0 padding, 0 border): the Activate Emergency Lockdown button rendered as 12px red plain text; Admin Settings Save Changes / Reset to Defaults rendered as plain text; Trash Restore / Purge rendered as plain text. (a) Adding rules for `wl-btn` would mean owning a parallel button system that has to track every Splunk visual update — a long-term liability. (b) Same problem, plus it would require migrating from `btn-primary` (which IS shipped by Splunk and works) to `wl-btn-primary` (which we'd own forever). (c) Killing `wl-btn` fully aligns the app with Splunk's bundled visual language at the cost of a one-time search-and-replace in 9 sites in `control_panel.js`. Splunk's `.btn` rules are stable across major versions; we're piggybacking on them rather than maintaining a parallel system. The migration is class-rename only — handlers find by `#id`, not class — so click semantics are preserved. | Low — `git revert` the build-631 + build-632 commits, redeploy at build 633. The `.wl-btn-locked` class is preserved so the approval-lock UX continues to work. **Maintenance rule**: do NOT introduce new `.wl-btn*` classes. Use Splunk-bundled `btn` / `btn-primary` and the four custom `.btn-success` / `.btn-danger` / `.btn-warning` rules in `whitelist_manager.css` (which we own and won't drift). When adding a NEW custom button colour, add it as `.btn-<name>` (a Bootstrap-style extension) not as `.wl-btn-<name>` (a parallel system). | |
| 33 | +| 2026-05-01 | **Drop light-theme support; force dark-only theme (build 637).** | (a) Keep the `:root` light + `body.wl-dark` override pattern and finish wiring light-mode tests; (b) force `wl-dark` always but leave the parallel CSS structure (~70 lines of dormant light vars); (c) collapse to single `:root` (dark only), remove `body.wl-dark` overrides, simplify `detectDarkTheme()`. | The app has been dark-first for its entire history. Light-mode paths were half-implemented: `body.wl-dark > .wl-modal-overlay` re-tightened `--wl-bg` for modals (duplicate of the dark vars in the main block), several inline-styled banners assumed dark bg, and there are no E2E tests for light mode. The build-636 light-theme test surfaced this — toggling `wl-dark` off via JS produced a half-converted state (body white, panels still dark, dropdowns half-transitioning) that was worse than either pure theme. (a) finishing the light path is several hours of work for zero current users (open-source target, no paying customer demanding it). (b) is reversible-but-half-done — keeps the light infrastructure as a tech-debt anchor that future contributors will trip over. (c) deletes the parallel system entirely: collapses `:root` (light defaults) and `body.wl-dark` (dark overrides) into a single `:root` block, removes the modal-overlay duplicate, and simplifies `detectDarkTheme()` to unconditionally apply `wl-dark`. The 19 existing `.wl-dark X` selectors stay as harmless redundancy (they always match because the class is always applied) — they can be flattened to plain `X` selectors in a follow-up cleanup. | Medium — re-introduce `:root` light vars (40 lines), re-add `body.wl-dark { ... }` wrapper around the dark vars, restore the brightness-based check in `wl_ui.js :: detectDarkTheme()`. Estimated 30 min if a customer ever requests light theme. **Maintenance rule (theme decisions for vanilla-JS apps)**: do NOT half-implement theming. Either commit fully (every panel uses CSS vars, every modal tested in both modes) or commit to one. The half-built state always produces visible jarring states when something goes wrong — and "something goes wrong" includes any contributor adding a new panel without remembering to use vars. | |
| 34 | +| 2026-05-01 | **Kill `.btn-success` taxonomy too; collapse all green buttons to Splunk-bundled `.btn-primary` (build 635).** | (a) Keep `.btn-success` as a parallel rule with our own muted shade; (b) override Splunk's `.btn-primary` to our muted shade so all greens match; (c) migrate every `.btn-success` callsite to `.btn-primary` and delete the rule. | The build-634 desaturation pass kept `.btn-success` muted (`#388e3c`) while `.btn-primary` rendered Splunk's vivid `#1a8929`. User-facing result: "+ Add Row" / "+ Add Column" (btn-primary) and Save Changes / Approve (btn-success) had two slightly-different greens side-by-side on the same toolbar — visible inconsistency that the user flagged. (a) Maintaining a parallel rule has the same long-term liability as `.wl-btn` did (see 2026-05-01 prior entry). (b) Overriding Splunk's bundled `.btn-primary` is risky — it could affect Splunk-native UI elsewhere on the dashboard (Edit / Export / Cancel buttons in Splunk modals). (c) is the same playbook as the wl-btn migration: 5 callsites in 4 files, single-line search-and-replace per file, zero handler changes (handlers find by `#id`, not class). | Low — re-add the `.btn.btn-success` rule to `whitelist_manager.css` and revert the 5 callsite edits. The `.btn.btn-danger` and `.btn.btn-warning` muted rules (also build 634) are unaffected — those colours have no Splunk bundled equivalent so we keep owning them. **Maintenance rule (search-and-replace as a workflow)**: when removing a parallel CSS taxonomy, the operation is a class-rename via grep, NOT a refactor. The pattern: (1) `grep btn-success` for every callsite, (2) Edit each callsite's class string in place, (3) delete the now-orphan CSS rule, (4) verify no callsites remain. Components-based apps would centralize this in one template; vanilla-JS Splunk apps render HTML strings inline per module, so consistency relies on grep + discipline. When facing a similar audit (ANY parallel-CSS-system removal), reach for the search-and-replace workflow first; resist the urge to "abstract" or "centralize" because there is no shared template layer to put the abstraction in. | |
| 35 | +| 2026-04-29 | **Declare security-hardening track CLOSED at build 629 (round 9).** No further proactive hardening rounds. Future inbound work that does NOT match a re-opening signal (CVE, production incident, external audit finding, Q3 routine surfacing major-version compat work, or methodology shift to a fresh fuzz surface) is feature work, not hardening work. | (a) Continue with rounds 10+ at the same cadence; (b) move to a 6-month re-audit cycle with no per-round structure; (c) formal closure with explicit re-open criteria. | 9 rounds covered every reasonable security gap. Round 8 fuzz found 0 bugs; round 9 was pure housekeeping with no `app.conf [install] build` bump — the natural diminishing-returns signal. CI gates (4 Semgrep rules + doc-drift + quarterly pip-audit + unit tests on every PR), live FIM monitoring (15s + ~2s paths + dual-store baseline), recurring audits (Q3 2026 version-pinning routine), and Sigstore signing of releases now make the system self-sustaining without further rounds. Continuing past clear diminishing returns risks (a) hardening fatigue masking real signal, (b) churn-without-substance erosion of the changelog's signal-to-noise, (c) future contributors expecting a "round N" cadence and queuing low-value work to fit. Closing prevents this. | Low — any re-opening signal (CVE, incident, audit finding, methodology shift, major-version compat) immediately re-opens with a new round entry. Closing is a status declaration, not an architectural change. The closing summary at the top of `CHANGELOG.md` is the canonical artifact; future contributors read it before queuing security work. **Maintenance rule**: do NOT propose "round 10" by default. The next round must be triggered by an inbound signal, not by schedule. | |
| 36 | +| 2026-05-11 | **E2E tests CI-gated via TWO workflows.** `e2e-smoke.yml` runs ~3 non-destructive E2E flow tests on every PR (~5 min). `e2e-full.yml` runs the entire 20-test E2E suite (destructive + visual regression + concurrency + stress) on a nightly UTC schedule and via manual `workflow_dispatch`. Smoke selection: `test_trash_traversal.cjs` + `test_rate_limit_burst.cjs` + `test_control_panel_long_content.cjs` — all UI-driven, no destructive helpers, exercise the REST → frontend stack end-to-end. The destructive `WL_TEST_HARNESS=1` env var is set only inline per nightly step, never globally. | (a) Full CI on every PR (run all 22 on each PR); (b) tag-gated (PR label `e2e` to run any E2E); (c) manual `workflow_dispatch` only with no nightly; (d) status quo (E2E tests never run in CI, only ad-hoc on developer machines). | (a) is rejected because 20 tests × ~30-60s + Playwright + container setup = 30+ min added to every PR, multiplied across every contributor push. Rate-limit collisions in the integration suite (a documented Ring 3 finding) would be worse for E2E since the destructive E2E tests deplete shared counters faster than the integration suite. (b) shifts the gating decision to contributors who may forget the label — over time the signal degrades to "nobody runs E2E in CI". (c) is what (d) effectively becomes: opt-in tools get used heavily for a month then forgotten. (d) is the prior state — 20 tests with real bug-finding signal lived entirely outside CI, so PR-introduced regressions had to wait for a developer's manual run to surface. The chosen two-workflow shape preserves PR feedback latency (smoke <5 min) while ensuring the full suite runs at least daily; failures in the nightly run upload artifacts and surface in the GitHub Actions summary the next morning. Mirrors the standard test-pyramid principle: heavier tests run less frequently but never skip. | Low — to revert to "full E2E on every PR": change `schedule` and `workflow_dispatch` to `pull_request` in `e2e-full.yml` and delete `e2e-smoke.yml`. To revert to status quo: delete both workflow files. The two-workflow shape is intentional separation; merging them would re-introduce the latency cost (a) was rejected for. **Maintenance rule (adding a new E2E test)**: by default, NEW tests go to the nightly full workflow. Only promote to smoke after the test has been stable in nightly for ≥2 weeks AND a smoke slot opens up (we cap smoke at ~5 tests to keep PR latency <5 min). If you promote a test to smoke, document the choice in the workflow file's comments and remove an existing one if the cap is hit. | |
| 37 | + |
| 38 | +**When a past decision is reversed**: add a NEW row with the new choice and a "Why reversed: see <YYYY-MM-DD> row" note. Do NOT delete or edit the old row — it is evidence for the next person who proposes the same losing option. |
0 commit comments