Repository navigation
docs(eventing): Phase 4 design — enrollment, and asking a human for permission - #897
Conversation
…ermission Two gaps Phase 2 and Phase 3 leave, which are the two halves of one idea: the notification channel is bidirectional. Phase 3 §4.2 derives an unguessable per-user ntfy topic and §4.3 provisions the reader account, but both are operator-side — a user who signs in with GitHub has no way to learn their own topic, and the token is printed to the *operator's* terminal. And every notification so far has been an announcement: Phase 3 §5.1 fixes each agent's permission mode and tool allowlist before the run, so an agent that needs a permission it was not granted cannot ask and a human cannot answer. There is no approval vocabulary anywhere in the tree. That second gap is what makes Phase 3's triggers risky to enable. §7.4 concedes a trigger's rendered prompt is attacker-influenced text and §5.1 answers with default-deny tools; default-deny with no escalation path means the operator's rational move is to widen the allowlist permanently for a capability needed once. An approval path is what keeps a narrow allowlist narrow. §2 is the measured part, and the rest depends on it Phase 2 §8.6's rule earned its keep twice. Against the CLI: a PreToolUse hook blocks 75 s with the tool not running (93 s wall clock), honours a per-hook "timeout": 120, refuses on `deny`, and delivers permissionDecisionReason to the agent — which then explained the refusal in its answer. `allow` runs the tool. Two negative findings, both attractive wrong turns. `--permission-prompt-tool` does not exist; the flag is `--permission-prompts <host|none>` and its own help defines `host` as the SDK host, so it needs the Agent SDK in-process — as does `canUseTool`. EventRunner spawns a subprocess, so the hook is the only mechanism available to it. And `plan` mode never attempts the guarded call at all: the agent wrote a plan and reached for ExitPlanMode, so the hook adjudicated nothing. Phase 3 §5.1 proposes `plan` as the safe default; for an agent subject to approval it is the wrong mode. The hook needs no new plumbing — AgentSpec already carries `settings_file`, resolved under the agent dir by `_resolve_under` and passed as `--settings`, and Phase 3 §5.2 already requires that directory be unwritable by the agent. The control that matters most Phase 3 §4.4 MACs `userkey|corr|exp`, which is right for continuing a conversation. Authorising an *action* has to bind the action: the approval key covers a digest of the canonical tool input, and the hook re-checks that digest before returning `allow`. Without it a key that approved `echo hi` could be replayed against a rewritten command on the same approval id. The verdict is a row read from the store, never an attribute off the event — Phase 2 §8.5.4, whose cautionary tale is group-member push suppression being defeated by omitting the attribute it reads. Once-only is a guarded UPDATE on `decided_utc IS NULL`, the pattern mark_member_finished already uses. A timeout is recorded as `timeout`, not `deny`: "nobody was awake" and "a human said no" need different fixes, and collapsing them hides which is happening. The budget inequality is pinned, because a killed hook is a non-blocking error — execution proceeds, i.e. the tool runs unapproved. Three ntfy action buttons is a hard ceiling and `_publish` already fills it, so an approval cannot be the result notification with a button added. §9 answers Phase 2 §8.5's five questions before this ships, including the hole that remains: the hook runs in the pod the agent runs in, so the mitigation is a filesystem permission rather than a cryptographic control. Design only — no code, no manifests, no tests, matching #892/#893 and the convention that a phase design lands as a reviewable record first. Nothing in it is implemented; §11 orders the tasks behind Phase 3's ntfy isolation and capability key. Not verified: no cluster access, and §2 was measured on CLI 2.1.270 while Dockerfile-eventrunner-claude pins 2.1.278 — §10 says so, and §9.1's pinned-CLI test is the first thing to build. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
|
Two findings came out of reviewing how identity reaches ntfy in this design. Both are Phase 3 defects rather than Phase 4 ones, so they are filed separately per
Neither blocks this PR — it is a design record and both findings are about the code it builds on. Flagging because #899 in particular bears on whether the approval channel in §5 should treat the ntfy topic as an authorization boundary at all. |
aslom
left a comment
There was a problem hiding this comment.
Summary
A design-only phase record that holds up under the claim-check its own §10 invites. I
verified every code reference with a line number, every file-existence claim, every count,
and the numeric constants — all of them are exact, including the three ntfy action labels,
the 5 if phase == "error" else 3 priorities, MSG_CAP = 3500, CLAUDE_VERSION=2.1.278,
and the sessions.sqlite schema-comment convention. §2 being measured rather than read is
what makes the rest of the document worth trusting, and the two negative findings
(--permission-prompt-tool absent, plan never attempting the guarded call) are the kind
that usually go unrecorded. §5.4's input-digest binding is the right control and §9.4
names the hole it does not close. Approving.
Five findings, none blocking: two duplicated-word typos from line wrapping, one real gap in
§5.7's budget inequality, and two consistency nits.
The one worth acting on before T8 is §5.7. The inequality pins
ER_APPROVAL_TIMEOUT_S < per-hook timeout < turn wall clock, and §6 separately names the
ER_DRAIN_TIMEOUT_S / terminationGracePeriodSeconds budget as the reason an unbounded
wait is unavailable — but never connects the two. The current defaults are fine (110 < 570
< 600, with a comfortable margin), so nothing is broken; the gap is that the one knob an
operator will reach for, when humans complain the approval window is too short, has
nothing pinning it below the drain budget. §7.2's description of
ER_APPROVAL_TIMEOUT_S repeats only the per-hook bound.
Author: mrsabath (MEMBER — maintainer)
Areas reviewed: Docs (2 files, +723/-0); cross-references and code claims verified
against pr897 @ dc1d432, which sits directly on current main (8ab8b30)
Agent/IDE config (.claude/.vscode): none — supply-chain gate clean, no secrets in diff
Commits: 1 commit, signed-off: yes (DCO passing)
CI status: passing — all 12 checks green
Claim-hygiene check, for the record
Ran the §10 / #892 check mechanically. Of the 95 §N.N references in the document (126
counting top-level §N too), the only
ones that do not resolve to an existing heading are the three Phase 1 §1.1 citations
(§1, §2.1, §3.4) — DESIGN_PHASE1.md has no §1.1 heading; the pure-Python rule is a
bullet inside its §1.
This is not a defect in this PR. §1.1 is how the whole document set names that rule
already: DESIGN_PHASE1.md cites its own §1.1 three times, and so do DESIGN_PHASE2.md,
DESIGN_PHASE3.md (×6), IMPLEMENTATION_REPORT1.md (×3) and KUBECON_NA_2026.md. This
document follows the convention correctly. Worth noting only because the PR description
states "Every §N.N reference resolves to a heading that exists" slightly more broadly
than holds — the document's own §10 wording ("here or in the Phase 2/3 document it names")
is accurate as written, and deliberately scoped. A one-line ### 1.1 Pure-Python discipline in Phase 1 would retire the whole class, but that is a separate PR.
Everything else reconciled: §5.9's three actions against _publish's three, §9.3's four
cases, §6's three steps, T1–T10 contiguous, and §9.3's characterisation of Phase 2 §8.1's
worst finding against that document's actual text.
Review: #897 (approved), five inline comments — two substantive, three nits. - §5.7's budget chain now carries §6's bound: the hook's wait, not just the turn, fits inside ER_DRAIN_TIMEOUT_S < terminationGracePeriodSeconds. The paragraph says why that link matters — ER_APPROVAL_TIMEOUT_S is the knob an operator raises when two minutes is not enough, and at 600 it outlives the drain budget, so a rolling update lands a SIGKILL mid-approval and §6's late-approval path replays a turn whose partition was taken away mid-flight. The startup check prints all five values; test_manifests.py's drain test is named as gaining the one-more-assert (ER_APPROVAL_TIMEOUT_S < ER_DRAIN_TIMEOUT_S, in the rendered ConfigMap). - §7.2's ER_APPROVAL_TIMEOUT_S row: "below the per-hook timeout AND below ER_DRAIN_TIMEOUT_S", pointing at §5.7's full chain. - §5.6 gains the missing 503 row (EB_CAPABILITY_SECRET_PATH empty, §7.1), distinguishing it from 404: the endpoint exists, the remedy is the operator's. - §3.4 and §6: dropped the duplicated "Phase 1" / "Phase 3" across the line wraps. - §9.5's VERIFY marker: dropped the ```markdown fence so both instances render the same (invisible HTML comments); grep-ability unchanged. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
Phase / scope
DESIGN_PHASE2.md(identity) andDESIGN_PHASE3.md(tenancy, ntfy isolation).agentdocsindex. No code, no manifests, no tests.Adds
eventing/agentdocs/DESIGN_PHASE4.md. Two gaps Phase 2 and Phase 3 leave, whichturn out to be the two halves of one idea — the notification channel is
bidirectional.
The two gaps
Nothing connects a GitHub login to a phone. Phase 3 §4.2 derives the unguessable
per-user ntfy topic and §4.3 provisions the reader account, but both are operator-side
(
k8s_tenant.py,kubectl exec), and §4.3 has the per-user token "handed to the useronce, at provisioning time" — printed to the operator's terminal. A user who has
signed in with GitHub still has no way to learn their own topic. That is a provisioning
story, not an enrollment story, and the operator is a required participant in every
signup.
Permission is decided before the run, and cannot be revisited. Phase 3 §5.1 fixes
each agent's
permission_modeand tool allowlists — "the sandbox" — in theAgentSpec.There is no approval vocabulary anywhere in the tree or in any design document: an
agent that needs a permission it was not granted cannot ask, and a human cannot answer.
The second gap is what makes Phase 3's triggers risky to enable. §7.4 concedes a
trigger's rendered prompt is attacker-influenced text and §5.1 answers with default-deny
tools — but default-deny with no escalation path means the operator's rational move is to
widen the allowlist permanently, for a capability needed once. An approval path is what
keeps a narrow allowlist narrow.
§2 is the measured part, and the rest depends on it
The whole design rests on one question: can a tool call be paused mid-run, in headless
claude -p, until an external system answers? If not, this is a notification after thefact rather than a gate. Phase 2 §8.6's rule applied — and it earned its keep twice,
because the documentation was wrong about which flag to use and I mispredicted the
permission mode.
Measured against the CLI,
--output-format stream-json --verbose --max-turns 2 --settings <file>:PreToolUsehook fires in-p(non-interactive) mode"timeout": 120honouredpermissionDecision: "deny"→ tool never executespermissionDecisionReasonreaches the agent as the tool resultpermissionDecision: "allow"→ tool executesecho HELLO_FROM_TOOL→HELLO_FROM_TOOLTwo negative findings, recorded because each is an attractive wrong turn:
--permission-prompt-tooldoes not exist on this CLI. The flag is--permission-prompts <host|none>, and its own help text defineshostas "the SDKhost" — it needs the Agent SDK driving the session in-process, as does
canUseTool.EventRunner spawns a subprocess, so the hook is the only mechanism available to it.
That is a constraint, not a preference.
planis the wrong mode for an agent subject to approval, and Phase 3 §5.1proposes it as the safe default. Measured: under
planthe agent wrote a plan file andreached for
ExitPlanModeinstead of attempting the guarded tool, so the hookadjudicated nothing — the call never happened.
The hook also needs no new plumbing:
AgentSpecalready carriessettings_file,resolved under the agent dir by
_resolve_underand passed as--settings, and Phase 3§5.2 already requires that directory be unwritable by the agent ("an agent definition the
agent itself could rewrite is not a policy, it is a suggestion" — that sentence is the
whole security argument for where the hook lives, and it was already written down).
The control that matters most, which Phase 3 §4.4 does not have
Phase 3 §4.4 MACs
userkey|corr|exp. That is right for continuing a conversation,where the only thing to bind is the correlation. Authorising an action has to bind
the action: the approval key covers a digest of the canonical tool input, and the hook
re-computes that digest before returning
allow. Without it, a key that approvedecho hiis replayable against a rewritten command on the same approval id.Three more decisions that follow the repository's existing discipline rather than
inventing anything:
§8.5.4, whose cautionary tale is group-member push suppression being defeated by
omitting the attribute it reads.
UPDATE … WHERE decided_utc IS NULLonrowcount == 1, thepattern
mark_member_finishedandcomplete_groupalready use. In SQL rather than inmemory, for the reason those are.
timeout, notdeny. "Nobody was awake" and "a human saidno" need different fixes, and collapsing them hides which is happening. §5.7 also pins
the budget inequality, because a killed hook is a non-blocking error — execution
proceeds, i.e. the tool runs unapproved. That is the one failure direction this
phase cannot have.
Two constraints that ruled out the obvious shapes
_publishalready fills it (Openhistory, Raw CloudEvents, Continue…). So "add an Approve button to the result
notification" silently drops one. An approval is its own notification shape.
ER_DRAIN_TIMEOUT_Sbudgettest_manifests.pyalready pins, and KEDA will not scale itaway mid-run. Hence §6: deny on timeout, keep the approval row alive, and let a late
approval start a new turn scoped to the recorded input digest. The cost is honest (the
agent redoes that work) and it trades a bounded cost for an unbounded one.
§9 answers Phase 2 §8.5's five questions before this ships
Including the hole that remains, stated rather than discovered: the hook runs in the pod
the agent runs in, so an agent with an unexpected write primitive over
/etc/rossoctl/agentsdisables its own guard. The mitigation is a filesystem permission,not a cryptographic control.
§9.3 tabulates all four half-configured cases, and the one that matters is deliberately
the opposite of Phase 2 §8.1's worst finding: guarded tools with EventBridge unreachable
→ the hook denies, loudly. Both sides fail closed, and both say so.
Not verified, and not claimed
same place docs(eventing): KubeCon NA knowledge base — abstract, status, plan, slides #893 does, rather than implying observation.
Dockerfile-eventrunner-claudepins 2.1.278. The contract is expected to holdacross that gap and has not been confirmed on 2.1.278 — which is exactly what
§9.1's test is for, and T10 is the first thing to build.
eventbridge/capability.pyis not in the tree andNtfyPublisheris still single-topic, so §11 orders the tasks behind Phase 3's ntfyisolation and capability key.
Verification
Design-only, so what was verified is the claims, per Phase 2 §8.6:
§N.Nreference resolves to a heading that exists, and is phase-qualified whereambiguous —
§4.2,§4.3,§4.4,§8.1and§8.3all exist in both Phase 2 andPhase 3, so a bare reference is genuinely ambiguous. Checked with a script, then
deleted.
§6's three steps, T1–T10 contiguous) — docs(eventing): Phase 2 — make §8's count checkable, and correct one mechanism #892's lesson.
from
ntfy.py's constants.corrected text. No Python touched, so
ruffis unaffected.Open question for review
§3.4 records a recommendation rather than a decision: the terminal QR code for
enrollment would mean ~150 lines of hand-rolled encoder forever (Phase 1 §1.1 forbids
qrcode/Pillow). I recommend shipping thentfy://deep link as text first and addingthe QR only if a demo shows the link is not enough. Worth a second opinion before T9.