Skip to content

Record what applying the A8 ruleset settled, including the verified actor id - #44

Merged
404SecNotFound merged 1 commit into
mainfrom
claude/cohaera-third-security-review-oaa6dd
Aug 30, 2026
Merged

Record what applying the A8 ruleset settled, including the verified actor id#44
404SecNotFound merged 1 commit into
mainfrom
claude/cohaera-third-security-review-oaa6dd

Conversation

@404SecNotFound

@404SecNotFound 404SecNotFound commented Aug 22, 2026

Copy link
Copy Markdown
Owner

What this changes, and why

Docs only — docs/OUTSTANDING.md. The A8 entry described the live application as pending and carried an unverified guess. Applying it for real answered four open questions, recorded as measurements rather than left to be re-derived.

The one flagged as unverified in #42:

question answer
Is actor_id: 5 / RepositoryRole the repository admin? Yescurrent_user_can_bypass returned always for the owner

Three more from the same run:

  • An invalid field aborts the whole PUT. The evaluate attempt returned 422 and changed nothing, so a bad apply fails closed rather than half-applying. That is the property the disable-verify-enable procedure leans on, now demonstrated rather than assumed.
  • A one-field PUT preserves the other rules. -f enforcement=… alone did not wipe the rule set.
  • The bypass cannot be confirmed from an automation session at all. GitHub omits bypass_actors for callers without admin, and current_user_can_bypass answers for the calling token — never for automation, always for the owner.

That last one is the important one. An automation session can read enforcement, the rules and the required checks, and cannot read the bypass — so "CI confirmed the ruleset" can never cover the half that decides whether main is mergeable. Only a maintainer can check it.

It also corrects something I got wrong in session: I earlier reported the live bypass_actors as [], reading through a .get() default. The key may simply have been absent for my token. The conclusion happened to be right, but the method could not have distinguished "empty" from "not visible to me" — precisely the confusion this repository exists to refuse.

And a live application is not proved by a successful merge. Merging with --admin while enforcement is disabled succeeds for the wrong reason: the flag is a no-op and nothing was gated. Read enforcement and updated_at back and confirm the timestamp moved. A re-enable that silently never reached GitHub looks identical to one that worked — that happened once during this application and was caught only by comparing timestamps.

Evidence

  • The defect was reproduced first — n/a in the usual sense: this records observations from a live application. Every row is a fact observed during it, traceable to API output seen in session.
  • I mutated the fix — n/a, nothing executable is added. Docs only, no assertion to mutate.
  • python -m pytest tests/ -q passes.
  • python tests/test_evasion.py — 34/34.
  • ruff check src tests eval tools passes.
  • python tools/readme_facts.py --check passes — no count moved on this base.
  • python tools/verify.py16 passed, 0 failed, 0 not evaluated.

If this changes detection / an evasion

  • n/a for both — documentation only, no src/ changes.

Anything a reviewer should push back on

  • None of this is enforced by a test, and the natural test — diffing the committed ruleset against the live one — is now known to be impossible from CI, because the token cannot read bypass_actors. A partial drift check covering everything except the bypass is possible; it might also give false comfort on exactly the field that matters. Worth a decision rather than a default.
  • actor_id: 5 is verified for this repository and this account. I have not established that the mapping is a documented GitHub constant rather than an implementation detail — treat it as "confirmed here", not "known universally".
  • The A8 entry is long now, reading more like a runbook than a checklist item. Deliberate, given how close this came to locking main, but a fair call if you would rather the procedure moved to its own page.

Note on ordering

#45 also touches the derived doc word count. Whichever of the two merges second will need one regeneration commit — the count is 115,000 on either base alone and 116,000 with both. Not a conflict in the git sense; the checker will simply ask for readme_facts.py --write. I will handle it on whichever lands second.

The committed ruleset guessed that actor_id 5 with RepositoryRole is the
repository admin, and said so. The first live application answered it:
current_user_can_bypass came back "always" for the owner. Recorded as
measured rather than left as a guess somebody has to re-derive.

Three other answers from the same run, each of which was an open question
when the procedure was written:

An invalid field aborts the whole PUT -- the evaluate attempt returned 422
and changed nothing, so a bad apply fails closed rather than half-applying.
A one-field PUT preserves the other rules. And the bypass cannot be
confirmed from an automation session at all: GitHub omits bypass_actors for
callers without admin, and current_user_can_bypass answers for the calling
token, reading "never" for automation and "always" for the owner.

That last one is the important one and it is now written down. An
automation session can read enforcement, the rules and the required checks,
and cannot read the bypass -- so "CI confirmed the ruleset" can never cover
the half that decides whether main is mergeable.

Also records that a successful merge does not prove a live application.
Merging with --admin while enforcement is disabled succeeds for the wrong
reason: the flag is a no-op and nothing was gated. Read enforcement and
updated_at back and check the timestamp moved, because a re-enable that
never reached GitHub looks identical to one that worked. That happened once
during this application and was caught only by comparing the timestamp.

Docs only.
@404SecNotFound
404SecNotFound force-pushed the claude/cohaera-third-security-review-oaa6dd branch 2 times, most recently from 35314f6 to 75cf9b9 Compare August 30, 2026 02:07
@404SecNotFound 404SecNotFound changed the title Record what applying the A8 ruleset settled, including the verified actor id Record what applying the A8 ruleset settled, and correct observra's rule engine signature Aug 30, 2026
@404SecNotFound
404SecNotFound force-pushed the claude/cohaera-third-security-review-oaa6dd branch from 75cf9b9 to 709fc58 Compare August 30, 2026 02:43
@404SecNotFound 404SecNotFound changed the title Record what applying the A8 ruleset settled, and correct observra's rule engine signature Record what applying the A8 ruleset settled, including the verified actor id Aug 30, 2026
@404SecNotFound
404SecNotFound merged commit a52da61 into main Aug 30, 2026
18 checks passed
404SecNotFound added a commit that referenced this pull request Aug 30, 2026
#44 landed while this was open. The count is 115,000 on either base alone
and 116,000 with both, so it drifted the moment the two met -- exactly the
shared-derived-count collision noted when these were split. Regenerated by
tools/readme_facts.py, not typed.
404SecNotFound added a commit that referenced this pull request Aug 30, 2026
The owner confirmed #44 landed through the web UI merge button. The entry
had said the UI "or gh pr merge --admin" work "because they take a different
path" -- and only the 405 was ever measured. The rest was inference.

Replaced with a table of what was actually observed per route: REST refused
with 405, web UI works, gh pr merge --admin NOT TESTED. Its one success on
this repository was under enforcement: disabled, where the flag is a no-op,
so it proves nothing about behaviour under an active ruleset.

Noted in the entry that the earlier wording was the same substitution the
paragraph beneath it warns against -- asserting how a control behaves
without watching it behave -- committed while writing the warning. That is
worth leaving on the page rather than quietly fixing, because the failure
mode is evidently easy enough to repeat inside the sentence describing it.

Pushed while CI was still running on this branch rather than after it went
green: two commits have already been lost to squashes here by arriving after
a run finished.

Docs only. No count moved.
404SecNotFound added a commit that referenced this pull request Aug 30, 2026
* Correct observra's rule engine signature, and say who owns it

Read at c4d036b rather than recalled. Three things were wrong or
incomplete on the front page, and one of them is the kind of error that
ends a conversation.

THE SIGNATURE WAS WRONG. The page printed

    evaluate_rules(event_type: str, data: dict) -> list[str]

against an actual

    def evaluate_rules(event_type, data) -> dict[str, Any]

which returns {"triggered_rules": [...], "max_severity": ...}. Printing
somebody else's function signature and getting the return type wrong is
not a small thing when the whole argument is about reading evidence
carefully.

THE OWNERSHIP WAS NOT ON THE PAGE. POSITIONING.md already recorded that
Exabeam ships observra; the front-page comparison did not, and that is
where anyone actually reads it. The gap being described is in Exabeam's
own capture layer -- Apache-2.0, copyright Exabeam Inc., flagship project
in the open-agent-ai-security org, with an Exabeam sender in tree. Said
plainly now, because discovering it mid-conversation reads as either
sloppiness or as a competitive dig, and it is neither.

THE CLAIM ITSELF HOLDS, AND IS STRONGER THAN STATED. Every rule is a
lambda over one event-type string and one data dict; there is no session,
no accumulator, no history. That is structural, not a gap in the rule set.

Also documents the apparent counter-example, since a reader who opens the
file will find it. A Suspicious Tool Sequence rule exists and reads a
suspicious_sequence flag. The function that would set it,
detect_suspicious_sequence(), is defined once and called nowhere in the
repository -- not in src, not in tests -- so nothing sets the flag and the
rule cannot fire. Were it wired up it computes has_read and has_external
over the session's tool names: set membership with no notion of order, so
send_email then read_file scores identically to the reverse.

Caught by the R-20 guard while writing this: an earlier draft said "all
ten rules", and a spelled count next to a countable is exactly what that
test exists to refuse. Removed the count rather than adding an exemption.

Docs only. No src/ changes.

* Re-derive the doc word count after merging main

#44 landed while this was open. The count is 115,000 on either base alone
and 116,000 with both, so it drifted the moment the two met -- exactly the
shared-derived-count collision noted when these were split. Regenerated by
tools/readme_facts.py, not typed.
404SecNotFound added a commit that referenced this pull request Aug 30, 2026
…sed (#46)

* Record that the ruleset bypass is path-dependent, found by being refused

Missed by #44's squash: pushed to the branch after the merge had been cut,
so main carries the A8 findings without the one that only appeared when a
gated merge was actually attempted. Second time this has happened on this
repository, and the cause is the same both times -- a commit pushed after
CI ran is not in the squash.

The A8 entry described a verification procedure that had been written, run,
and passed. The first gated merge was refused anyway.

With the ruleset active, one approval required and zero reviews on the pull
request, PUT /repos/{owner}/{repo}/pulls/{n}/merge returns 405 "Pull Request
is not mergeable" for an owner whose current_user_can_bypass reads "always".
The same request reports "mergeable": true with "mergeable_state":
"blocked": no git conflict, held purely by the approval rule.

So current_user_can_bypass states an actor's ELIGIBILITY, not that the
bypass is honoured on the path being used. The REST merge endpoint enforces
the rule and offers no override parameter.

The earlier check was not wrong, it was narrow. It answered "is this actor a
bypass actor" when the question that mattered was "will this merge succeed".
Those came apart at the first opportunity, and the page now says so next to
the procedure that missed it.

Worth stating plainly because it is this project's own argument turned on
itself: a control verified through the interface that REPORTS on it, rather
than through the action it GOVERNS, is verified in the weakest available
sense. Cohaera exists to refuse exactly that substitution in agent
telemetry, and it took two attempts on this repository to notice it applied
here too.

Docs only. Word count regenerated, not typed.

* Name the merge route that was observed, and mark the other untested

The owner confirmed #44 landed through the web UI merge button. The entry
had said the UI "or gh pr merge --admin" work "because they take a different
path" -- and only the 405 was ever measured. The rest was inference.

Replaced with a table of what was actually observed per route: REST refused
with 405, web UI works, gh pr merge --admin NOT TESTED. Its one success on
this repository was under enforcement: disabled, where the flag is a no-op,
so it proves nothing about behaviour under an active ruleset.

Noted in the entry that the earlier wording was the same substitution the
paragraph beneath it warns against -- asserting how a control behaves
without watching it behave -- committed while writing the warning. That is
worth leaving on the page rather than quietly fixing, because the failure
mode is evidently easy enough to repeat inside the sentence describing it.

Pushed while CI was still running on this branch rather than after it went
green: two commits have already been lost to squashes here by arriving after
a run finished.

Docs only. No count moved.
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