Skip to content

docs: close out the security review, record the round 3 triage - #57

Merged
404SecNotFound merged 3 commits into
mainfrom
docs/review-closeout
Aug 3, 2026
Merged

docs: close out the security review, record the round 3 triage#57
404SecNotFound merged 3 commits into
mainfrom
docs/review-closeout

Conversation

@404SecNotFound

Copy link
Copy Markdown
Owner

Three documents, no code.

security-review-2026-08-response.md gets a status table at the top. Every
finding is now closed except F-08, which needs a decision rather than an
implementation, and F-14, which is parked on the same kind of decision: the
remaining advisories need vite 8 and vitest 4, both of which drop Node 18, and
the installer declares 18 as its floor.

It also records what a second, independent review found that the security review
did not. That review looked at transport, web and process safety and never at
whether the emitted telemetry means what it claims, which is the product's core
promise. Two of three vendors were rendering every successful REP-018
administrative login as a rejected one.

And it records the two defects neither review found, both surfaced by running
the software during a live lab session rather than by reading it. Both read to
the operator as "the web UI is broken".

round3-expansion-triage.md is new: a decision record for an externally
produced 17-technique expansion proposal. Nothing from it is implemented, and
the source document is deliberately not in this repository, because what belongs
here is the decision rather than the draft.

The report's claims about this codebase were checked against the code rather
than trusted, and two were wrong in ways that changed a conclusion. Its
citations could not be verified in that session, and three specific weaknesses
are recorded so they are treated as [Unverified] rather than quietly inherited.
Seven proposals are worth adopting, six need changes first, three are rejected
with reasons. The most valuable part of it was not a technique.

CLAUDE.md gains the five conventions this work established, including the
one that cost the most to learn: a golden-line test covering one verdict of a
two-verdict field is not a test of that field.

Review finding #1. ScenarioManifest recorded `vendor` and `duration`;
RunManifest recorded neither, and nothing recorded the rate actually in force or
what the socket really did. A manifest that cannot say which profile rendered it
is a weaker audit record than safety rule 5 implies, and the web layer had to
paper over the gap.

Four fields, all defaulted so manifests written before them still load, which is
the convention `pace` and `speed` already set:

- `vendor`      which dialect rendered these events
- `duration`    the --duration asked for, verbatim
- `rate`        the events-per-second ceiling in force
- `send_stats`  sends, bytes, errors, oversize

`send_stats` is None rather than a row of zeroes when a run had no collector,
because zeroes read like a send that delivered nothing. `event_count` counts
events rendered; `send_stats["sends"]` counts datagrams the kernel accepted, and
the two differing is the interesting case rather than an inconsistency.

Captured on every exit path, so a run that failed part-way still records what its
socket managed before it died.

Read with getattr rather than `emitter.stats`, because the emitter is an
injection point and several test doubles implement only send/close. Requiring the
attribute would make this record cost every future fake a field it does not
otherwise need. Four existing test doubles proved the point immediately.

875 py. black, ruff, mypy clean.
…UI leads with it

The catalog described what each technique EMITS in considerable detail and never
said what running it is meant to ESTABLISH. The detail panel filled that gap with
a generated sentence:

    Emits synthetic traffic:forward telemetry that exercises NDR-C2-001.

That is true of all 24 entries. It reads as specific, carries nothing that
distinguishes one entry from another, and so answers none of the only question
the screen exists to answer: which one do I run?

Each entry now carries an `objective`: one sentence saying what a detection
engineer is trying to prove by running it. They are grounded in what each
technique actually emits, not restatements of the name. The interesting ones are
the entries whose objective is not "find the bad thing":

- REP-017 proves a rule can notice an ABSENCE, resolver traffic stopping.
- REP-021 exists to be a false-positive source, so outbound scan rules can be
  shown not to fire on inbound perimeter noise.
- REP-024 proves a rule separates an unsanctioned relay from a sanctioned proxy
  behaving identically, where asset role is the only difference.
- REP-015 proves DNS exfil detection still works when query RATE is normal.

The objective renders above the templated sentence, in the reading colour, and
the template is demoted to the mechanical detail it always was. Ordering is
asserted, not just presence: the operator should read what it is for before what
it emits.

Guarded parametrized over the whole catalog rather than spot-checked, for the
reason the `--duration` work established: a field present on most entries is
worse than one present on none, because the operator learns to trust it. Four
guards: present, a real sentence rather than a label, not the name restated, and
all 24 distinct. That last one is the defect stated directly.

948 py + 139 fe. black, ruff, mypy, tsc clean.
Three documents, no code.

**security-review-2026-08-response.md** gets a status table at the top. Every
finding is now closed except F-08, which needs a decision rather than an
implementation, and F-14, which is parked on the same kind of decision: the
remaining advisories need vite 8 and vitest 4, both of which drop Node 18, and
the installer declares 18 as its floor.

It also records what a second, independent review found that the security review
did not. That review looked at transport, web and process safety and never at
whether the emitted telemetry means what it claims, which is the product's core
promise. Two of three vendors were rendering every successful REP-018
administrative login as a rejected one.

And it records the two defects neither review found, both surfaced by running
the software during a live lab session rather than by reading it. Both read to
the operator as "the web UI is broken".

**round3-expansion-triage.md** is new: a decision record for an externally
produced 17-technique expansion proposal. Nothing from it is implemented, and
the source document is deliberately not in this repository, because what belongs
here is the decision rather than the draft.

The report's claims about this codebase were checked against the code rather
than trusted, and two were wrong in ways that changed a conclusion. Its
citations could not be verified in that session, and three specific weaknesses
are recorded so they are treated as [Unverified] rather than quietly inherited.
Seven proposals are worth adopting, six need changes first, three are rejected
with reasons. The most valuable part of it was not a technique.

**CLAUDE.md** gains the five conventions this work established, including the
one that cost the most to learn: a golden-line test covering one verdict of a
two-verdict field is not a test of that field.
@404SecNotFound
404SecNotFound merged commit 4a98965 into main Aug 3, 2026
10 checks passed
@404SecNotFound
404SecNotFound deleted the docs/review-closeout branch August 3, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant