Skip to content

Commit 1824698

Browse files
author
wildleo91
committed
chore(housekeeping): round 9 — doc-drift, dead artifacts, PR-time anti-pattern gating (no build bump)
First round in the 552->629 hardening series with no build bump. All edits are repo housekeeping; no runtime behavior change. Appropriate signal that we're now in pure cleanup territory. Tier 1.1: fim_code_modified -> fim_file_modified doc drift fix. Round 8 verification showed the actual emitted action name is fim_file_modified; prose in 4 sites called it fim_code_modified. Fixed bin/wl_fim.py comments (2) and prior CHANGELOG round-7 + round-6 entries (2). The round-8 drift-discovery entry is left intact (it correctly reports both names). Tier 1.2: stale dist/ artifacts removed. wl_manager-1.0.0.spl and wl_manager-2.0.0.spl (+ .sha256 sidecars) predated build 406; current is 629. Empty dist/ now; package.sh writes fresh artifacts on next run. Tier 1.3: 17 root-level PNG screenshots gitignored via /*.png (root-only). Session debris from past Playwright runs no longer pollutes git status. Canonical product screenshots under docs/screenshots/ remain tracked. Tier 1.4: PR-time anti-pattern gating via Semgrep. New rule tests/semgrep/payload-from-flag-bypass-splunk.yaml catches at PR-review time what TestNoUnderscoreFlagPayloadBypass catches at test-run time. Pattern-mode (not taint-mode) because the anti-pattern is a structurally-illegitimate code shape, not a tainted dataflow. Writes (LHS of assignment) excluded via pattern-not-inside. Verified: 4/4 positive cases fire, 0/3 negative cases fire, 0 findings on current bin/ (existing code already clean per round 7 A1 cleanup).
1 parent 2af4da1 commit 1824698

5 files changed

Lines changed: 159 additions & 5 deletions

File tree

.gitignore

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ login-check
5353
*.log
5454
*.patch
5555

56+
# Session debris — screenshots taken during dev / Playwright runs end up
57+
# at the repo root and accumulate. Canonical product screenshots live
58+
# under docs/screenshots/ (still tracked because they don't match this
59+
# root-only pattern). Round 9 (2026-04-29) — was 17 untracked PNGs at
60+
# the root from past sessions, all session debris.
61+
/*.png
62+
5663
# OS files
5764
.DS_Store
5865
.DS_Store?

CHANGELOG.md

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,60 @@
22

33
All notable changes to this project will be documented in this file.
44

5+
## Unreleased — 2026-04-29 (build 629, no app changes)
6+
7+
### Round 9: housekeeping — doc-drift, dead artifacts, PR-time anti-pattern gating
8+
9+
No runtime behavior change. All edits are repo housekeeping that the
10+
prior 8 rounds accumulated. First round in the 552→629 series with no
11+
`app.conf [install] build` bump — appropriate signal that we're now
12+
in pure cleanup territory.
13+
14+
#### Fixed (doc drift only)
15+
16+
- **`fim_code_modified``fim_file_modified`** in `bin/wl_fim.py`
17+
comments (2 sites) and prior round-7 + round-6 CHANGELOG prose
18+
entries. Round 8 verification surfaced the drift: a search for
19+
`fim_code_modified` returned zero rows because the actual emitted
20+
action name is `fim_file_modified`. Code unchanged; only prose
21+
was wrong. The round-8 drift-discovery entry that DOCUMENTS the
22+
drift is left intact (it correctly reports both names).
23+
24+
#### Cleaned up
25+
26+
- **Stale `dist/` artifacts removed**: `wl_manager-1.0.0.spl` and
27+
`wl_manager-2.0.0.spl` (+ their `.sha256` sidecars). Both
28+
predated build 406 (current is 629). They were untracked
29+
(`dist/` is gitignored) but confused anyone running
30+
`package.sh` for the first signed release. Empty `dist/` now;
31+
`package.sh` writes fresh artifacts on next run.
32+
- **Root-level PNG screenshots gitignored**: 17 untracked PNGs at
33+
the repo root (e.g. `csv-loaded.png`, `stress-pending-table-build615.png`)
34+
from past dev/Playwright sessions polluted `git status`.
35+
`.gitignore` now has `/*.png` (root-only) so session debris
36+
doesn't accumulate in tracked-file status. Canonical product
37+
screenshots under `docs/screenshots/` remain tracked.
38+
39+
#### Added
40+
41+
- **PR-time anti-pattern gating via Semgrep**:
42+
`tests/semgrep/payload-from-flag-bypass-splunk.yaml`. Catches at
43+
PR-review time what
44+
`tests/unit/test_ascii_validation.py::TestNoUnderscoreFlagPayloadBypass`
45+
catches at test-run time. Earlier feedback in the dev loop = lower
46+
fix cost. Pattern-mode (not taint-mode) because the anti-pattern is
47+
a structurally-illegitimate code shape, not a tainted dataflow:
48+
any `payload.get("_from_*")` or `payload["_from_*"]` READ is wrong
49+
regardless of subsequent sanitization. Writes (LHS of assignment)
50+
are explicitly excluded via `pattern-not-inside`. Verified: 4/4
51+
positive cases fire, 0/3 negative cases fire, 0 findings on
52+
current `bin/`. Past incidents addressed:
53+
- Round 1-3: `_from_approval` reads in 4 action wrappers
54+
- Round 5: `_from_dual_approval` reads in dual-admin paths
55+
- Round 7 A1: 7 dead writes removed (writes were OK; reads were not)
56+
- `tests/semgrep/README.md` updated to document the new rule and
57+
why it's pattern-mode while the other three are taint-mode.
58+
559
## Unreleased — 2026-04-29 (build 629)
660

761
### Round 8: residue + recurring guards + supply-chain hardening
@@ -144,7 +198,7 @@ All notable changes to this project will be documented in this file.
144198
downstream customers. Tampering means a poisoned release ships
145199
without ever modifying runtime code in the container — supply-chain
146200
surface that was upstream of every other monitored path. FIM now
147-
alerts within ~15 s on any edit (`fim_code_modified` HIGH).
201+
alerts within ~15 s on any edit (`fim_file_modified` HIGH).
148202
- **Per-job `permissions:` blocks** on every CI workflow
149203
(`.github/workflows/{ci,release,semgrep,validate-and-package}.yml`).
150204
Each job declares its required scope independently of the
@@ -291,7 +345,7 @@ All notable changes to this project will be documented in this file.
291345
and `scripts/pre-commit-doc-drift.sh`. Tampering with these
292346
unsigned bash scripts (which perform privileged operations like
293347
clearing tamper flags or appending to the recovery log) now
294-
surfaces as a `fim_code_modified` event within ~15s.
348+
surfaces as a `fim_file_modified` event within ~15s.
295349

296350
#### Splunk version audit (preliminary)
297351

bin/wl_fim.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def _expand_globs(patterns):
159159
# KV records, append to recovery log). An attacker with file
160160
# write could modify them to skip the audit log append or to
161161
# silently no-op. FIM coverage means tampering surfaces as a
162-
# `fim_code_modified` event within 15s.
162+
# `fim_file_modified` event within 15s.
163163
os.path.join(APP_DIR, "scripts", "emergency_unlock.sh"),
164164
os.path.join(APP_DIR, "scripts", "reset_cooldowns.sh"),
165165
os.path.join(APP_DIR, "scripts", "fim_deploy_window.sh"),
@@ -169,7 +169,7 @@ def _expand_globs(patterns):
169169
# `package.sh` could ship a poisoned release without ever
170170
# touching the runtime code — supply-chain attack surface that
171171
# is upstream of every other monitored path. FIM coverage means
172-
# tampering surfaces as a `fim_code_modified` event within 15s.
172+
# tampering surfaces as a `fim_file_modified` event within 15s.
173173
os.path.join(APP_DIR, "scripts", "package.sh"),
174174
])
175175

tests/semgrep/README.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,19 @@ Splunk-adapted Semgrep rules that gate every PR via
1010
| `ssrf-splunk.yaml` | Splunk handler payload flowing into `urllib.request.urlopen` / `urllib.request.Request` / `requests.{get,post}` without a hardcoded-localhost prefix | ERROR |
1111
| `command-injection-splunk.yaml` | Payload reaching `subprocess.{run,Popen,call,check_call,check_output}(..., shell=True, ...)` without `shlex.quote` / `shlex.join` | ERROR |
1212
| `path-traversal-splunk.yaml` | Payload reaching `open`, `os.path.join`, `shutil.copy*`, `os.rename`, `os.remove`, etc. without going through one of our validation wrappers (`build_csv_path`, `resolve_csv_path`, `is_safe_filename`) or a path-canonicalization sanitizer (`abspath`, `realpath`, `Path(...).resolve()`, `basename`, `startswith` containment check) | ERROR |
13+
| `payload-from-flag-bypass-splunk.yaml` | Reads of `_from_*` flags from the user-controlled REST `payload` dict (e.g. `payload.get("_from_approval")` or `payload["_from_dual_approval"]`). Writes (LHS of assignment) are explicitly allowed because they're server-controlled mutations by replay code. | ERROR |
1314

14-
All three are **taint mode**: they follow data from source (our handler's
15+
The first three are **taint mode**: they follow data from source (our handler's
1516
payload accessors) to sink (dangerous calls) and only fire if no sanitizer
1617
breaks the flow.
1718

19+
The fourth (`payload-from-flag-bypass-splunk.yaml`) is **pattern mode** — it
20+
matches a structurally-illegitimate code shape rather than a tainted dataflow.
21+
It pairs with
22+
`tests/unit/test_ascii_validation.py::TestNoUnderscoreFlagPayloadBypass` for
23+
post-merge enforcement; the Semgrep rule fires earlier in the dev loop (PR
24+
review) so a regression is caught before reaching a human reviewer.
25+
1826
## Origin
1927

2028
Adapted from [`gadievron/raptor`](https://github.com/gadievron/raptor)'s
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# ─────────────────────────────────────────────────────────────────────
2+
# Semgrep rule: forbidden `_from_*` payload-bypass anti-pattern
3+
# ─────────────────────────────────────────────────────────────────────
4+
#
5+
# Round 9 (build 629, 2026-04-29). Catches at PR-review time what
6+
# `tests/unit/test_ascii_validation.py::TestNoUnderscoreFlagPayloadBypass`
7+
# catches at test-run time. Earlier feedback in the dev loop = lower
8+
# fix cost.
9+
#
10+
# What this catches:
11+
#
12+
# if payload.get("_from_approval"): # READ — bypass-bypass
13+
# skip_security_check = True
14+
#
15+
# if payload["_from_dual_approval"]: # READ — same anti-pattern
16+
# skip_security_check = True
17+
#
18+
# What this does NOT catch (legitimate writes by server-controlled code):
19+
#
20+
# replay_payload["_from_approval"] = True # WRITE, server-set
21+
# exec_payload.setdefault("_from_dual_approval", True)
22+
#
23+
# Why this anti-pattern is forbidden: the REST `payload` is the raw
24+
# user-supplied JSON envelope. Any remote caller can put
25+
# `{"_from_approval": true}` in the body. If the handler reads that as
26+
# a security flag, the caller can self-grant the bypass. The fix is
27+
# always the same: move the flag to a kwarg on the receiving function,
28+
# set only by server-side replay code.
29+
#
30+
# Past incidents (per `MEMORY.md` / `feedback_silent_exception_security.md`):
31+
# - Round 1-3: `_from_approval` reads in 4 action wrappers
32+
# - Round 5: `_from_dual_approval` reads in dual-admin paths
33+
# - Round 7 A1: 7 dead writes removed (writes are OK; reads are not)
34+
#
35+
# Pairs with: `tests/unit/test_ascii_validation.py::TestNoUnderscoreFlagPayloadBypass`
36+
# (same property, post-merge enforcement). Both layers must agree. If
37+
# this rule fires but the unit test doesn't (or vice versa), one of
38+
# them has drifted — investigate.
39+
40+
rules:
41+
- id: payload-from-flag-bypass-splunk
42+
message: |
43+
SECURITY: reading `_from_*` flags from the user-controlled
44+
`payload` dict is a security-bypass anti-pattern. Move this
45+
flag to a kwarg on the receiving function, set only by server-
46+
side replay code. See
47+
`tests/unit/test_ascii_validation.py::TestNoUnderscoreFlagPayloadBypass`
48+
for the regression test that enforces this post-merge.
49+
severity: ERROR
50+
languages: [python]
51+
metadata:
52+
category: security
53+
cwe: "CWE-285: Improper Authorization"
54+
owasp: "A01:2021 - Broken Access Control"
55+
confidence: HIGH
56+
paths:
57+
include:
58+
- bin/
59+
patterns:
60+
- pattern-either:
61+
# `payload.get("_from_...")` — always a read, no false-positive form.
62+
- patterns:
63+
- pattern: payload.get($KEY)
64+
- metavariable-regex:
65+
metavariable: $KEY
66+
regex: ^["\047]_from_
67+
- patterns:
68+
- pattern: payload.get($KEY, $DEFAULT)
69+
- metavariable-regex:
70+
metavariable: $KEY
71+
regex: ^["\047]_from_
72+
# `payload["_from_..."]` — could be read OR write. Exclude the
73+
# write forms (LHS of assignment) using `pattern-not-inside`
74+
# so server-controlled mutations by replay code stay clean.
75+
# `pattern-not` would not work here: the subscript node is a
76+
# CHILD of the assignment in the AST, not equal to the
77+
# assignment itself.
78+
- patterns:
79+
- pattern: payload[$KEY]
80+
- pattern-not-inside: payload[$KEY] = $V
81+
- pattern-not-inside: payload[$KEY] += $V
82+
- pattern-not-inside: payload[$KEY] -= $V
83+
- metavariable-regex:
84+
metavariable: $KEY
85+
regex: ^["\047]_from_

0 commit comments

Comments
 (0)