mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(relay): enforce E2E latch on command-path bodies
Command-kind events (WORKFLOW_DEF/TRIGGER, APPROVAL_GRANT/DENY) short-circuit at is_command_kind before the 15c ciphertext gate, so plaintext YAML, trigger inputs, and approval notes could be persisted into a latched (encryption_activated_at-set) DM channel — defeating the latch. The drift guard keyed off the narrow requires_h_channel_scope proxy, letting command kinds escape classification. Add a body-shape latch check (empty or NIP-44 v2, else reject fail-visible) at the command path, mirroring the 15c gate's invariant. The rule is kind-agnostic, so it cannot drift and catches plaintext smuggled into nominally-structured kinds. Repoint e2e_drift_guard to the real acceptance surface (required_scope_for_kind().is_ok() && !is_global_only_kind()) with a 4-bucket exactly-one classification. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -67,6 +67,73 @@ enum PersistResult {
|
||||
Inserted(sqlx::Transaction<'static, sqlx::Postgres>),
|
||||
}
|
||||
|
||||
/// Disposition of a command body submitted into a latched (E2E-encrypted)
|
||||
/// channel. Mirrors the 15c ingest gate's invariant — "private-channel content
|
||||
/// must be NIP-44 encrypted" — but for the command path, which short-circuits
|
||||
/// at `ingest.rs` BEFORE that gate (`is_command_kind` → `handle_command`), so
|
||||
/// command bodies never reach it. This is the choke-point equivalent.
|
||||
///
|
||||
/// The rule is body-shape, NOT per-kind: classifying by kind would re-create
|
||||
/// the drift-guard footgun this fix exists to close. A nominally-structured
|
||||
/// command that smuggles plaintext is caught for free.
|
||||
///
|
||||
/// - empty → Ok: structured commands (DM_OPEN/ADD_MEMBER/HIDE, and
|
||||
/// TRIGGER/APPROVAL with no inputs/note) legitimately carry no body.
|
||||
/// - valid NIP-44 v2 → Ok: an encrypted body is the encrypted boundary holding.
|
||||
/// - non-empty plaintext → Err: the leak — reject fail-visible.
|
||||
///
|
||||
/// `validate_nip44_v2("")` returns `Err(Empty)`, so empty MUST be special-cased
|
||||
/// here rather than delegated to the validator wholesale.
|
||||
fn latched_body_disposition(content: &str) -> Result<(), IngestError> {
|
||||
if content.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
buzz_core::observer::validate_nip44_v2(content).map_err(|_| {
|
||||
IngestError::Rejected("invalid: private-channel content must be NIP-44 encrypted".into())
|
||||
})
|
||||
}
|
||||
|
||||
/// Enforce [`latched_body_disposition`] when the resolved target channel is
|
||||
/// latched (`encryption_activated_at.is_some()`). A `None` channel or a
|
||||
/// not-found channel passes through (cannot be latched, mirrors the 15c gate's
|
||||
/// `ChannelNotFound` fall-through); a genuine DB error is fail-VISIBLE — it must
|
||||
/// not let plaintext slip into a latched channel.
|
||||
async fn enforce_latched_body(
|
||||
state: &Arc<AppState>,
|
||||
content: &str,
|
||||
channel_id: Option<Uuid>,
|
||||
) -> Result<(), IngestError> {
|
||||
let Some(ch_id) = channel_id else {
|
||||
return Ok(());
|
||||
};
|
||||
match state.db.get_channel(ch_id).await {
|
||||
Ok(channel) if channel.encryption_activated_at.is_some() => {
|
||||
latched_body_disposition(content)
|
||||
}
|
||||
Ok(_) => Ok(()),
|
||||
Err(buzz_db::DbError::ChannelNotFound(_)) => Ok(()),
|
||||
Err(e) => Err(IngestError::Rejected(format!("error: database error: {e}"))),
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve an approval's target channel via its workflow, then enforce the
|
||||
/// latched-body rule on the approval note. Approvals reference no `h` tag of
|
||||
/// their own — the channel is the workflow's (`get_workflow().channel_id`).
|
||||
/// A workflow with no channel (global) cannot be latched, so it passes through.
|
||||
async fn enforce_latched_approval_note(
|
||||
state: &Arc<AppState>,
|
||||
content: &str,
|
||||
workflow_id: Uuid,
|
||||
) -> Result<(), IngestError> {
|
||||
let channel_id = state
|
||||
.db
|
||||
.get_workflow(workflow_id)
|
||||
.await
|
||||
.ok()
|
||||
.and_then(|w| w.channel_id);
|
||||
enforce_latched_body(state, content, channel_id).await
|
||||
}
|
||||
|
||||
/// Persist a command event inside a transaction. Returns the OPEN transaction
|
||||
/// as an idempotency guard — if the event was already stored, `Duplicate` is
|
||||
/// returned and the handler skips execution.
|
||||
@@ -579,6 +646,10 @@ async fn handle_workflow_def(
|
||||
));
|
||||
}
|
||||
|
||||
// Latched-channel boundary: reject plaintext YAML into an E2E DM BEFORE
|
||||
// parsing — a 64KB plaintext body must not be parsed, only refused.
|
||||
enforce_latched_body(state, &event.content, Some(channel_id)).await?;
|
||||
|
||||
// 3. Parse YAML from event.content
|
||||
let (def, definition_json_str) = buzz_workflow::WorkflowEngine::parse_yaml(&event.content)
|
||||
.map_err(|e| IngestError::Rejected(format!("invalid: workflow YAML parse error: {e}")))?;
|
||||
@@ -685,6 +756,10 @@ async fn handle_workflow_trigger(
|
||||
));
|
||||
}
|
||||
|
||||
// Latched-channel boundary: trigger inputs (event.content, parsed as JSON
|
||||
// below) must be empty or NIP-44 in a latched channel — never plaintext.
|
||||
enforce_latched_body(state, &event.content, workflow.channel_id).await?;
|
||||
|
||||
// Persist the command event — returns open transaction
|
||||
let tx = match persist_command_event(state, event).await? {
|
||||
PersistResult::Duplicate => {
|
||||
@@ -860,6 +935,10 @@ async fn handle_approval_grant(
|
||||
// 4. Validate caller is authorized approver
|
||||
check_approver_spec(&approval.approver_spec, &self_hex)?;
|
||||
|
||||
// Latched-channel boundary: the approval note (event.content) must be empty
|
||||
// or NIP-44 in the workflow's latched channel — never plaintext.
|
||||
enforce_latched_approval_note(state, &event.content, approval.workflow_id).await?;
|
||||
|
||||
// Persist the command event — returns open transaction
|
||||
let tx = match persist_command_event(state, event).await? {
|
||||
PersistResult::Duplicate => {
|
||||
@@ -967,6 +1046,10 @@ async fn handle_approval_deny(
|
||||
// 4. Validate caller is authorized approver
|
||||
check_approver_spec(&approval.approver_spec, &self_hex)?;
|
||||
|
||||
// Latched-channel boundary: the approval note (event.content) must be empty
|
||||
// or NIP-44 in the workflow's latched channel — never plaintext.
|
||||
enforce_latched_approval_note(state, &event.content, approval.workflow_id).await?;
|
||||
|
||||
// Persist the command event — returns open transaction
|
||||
let tx = match persist_command_event(state, event).await? {
|
||||
PersistResult::Duplicate => {
|
||||
@@ -1147,3 +1230,66 @@ async fn resume_workflow_after_approval(
|
||||
.await;
|
||||
engine.finalize_run(run_id, result, existing_trace).await;
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The leak: a plaintext workflow YAML body in a latched channel must be
|
||||
/// REJECTED, not silently stored. Pre-fix this body reached `parse_yaml` and
|
||||
/// was persisted as plaintext — the CRITICAL leak this fix closes.
|
||||
#[test]
|
||||
fn test_latched_workflow_def_plaintext_yaml_is_rejected() {
|
||||
let yaml = "name: leak\non: manual\nsteps:\n - run: echo hi\n".repeat(1000);
|
||||
assert!(matches!(
|
||||
latched_body_disposition(&yaml),
|
||||
Err(IngestError::Rejected(_))
|
||||
));
|
||||
}
|
||||
|
||||
/// A plaintext approval note in a latched channel must be REJECTED.
|
||||
#[test]
|
||||
fn test_latched_approval_plaintext_note_is_rejected() {
|
||||
let note = "approved because the deploy looked fine to me";
|
||||
assert!(matches!(
|
||||
latched_body_disposition(note),
|
||||
Err(IngestError::Rejected(_))
|
||||
));
|
||||
}
|
||||
|
||||
/// Plaintext JSON trigger inputs in a latched channel must be REJECTED.
|
||||
/// JSON braces/quotes/spaces fall outside the base64 alphabet, so the strong
|
||||
/// validator refuses them.
|
||||
#[test]
|
||||
fn test_latched_workflow_trigger_plaintext_inputs_are_rejected() {
|
||||
let inputs = r#"{"env": "prod", "force": true}"#;
|
||||
assert!(matches!(
|
||||
latched_body_disposition(inputs),
|
||||
Err(IngestError::Rejected(_))
|
||||
));
|
||||
}
|
||||
|
||||
/// A structured command with no body (DM_ADD_MEMBER, an empty-inputs TRIGGER,
|
||||
/// a no-note APPROVAL) is ACCEPTED in a latched channel — the inverse-failure
|
||||
/// guard: the rule must not over-block legitimate empty-body commands.
|
||||
#[test]
|
||||
fn test_latched_empty_body_command_is_accepted() {
|
||||
assert!(latched_body_disposition("").is_ok());
|
||||
}
|
||||
|
||||
/// A genuinely NIP-44 v2 encrypted body is ACCEPTED in a latched channel —
|
||||
/// the encrypted boundary holding is the success path.
|
||||
#[test]
|
||||
fn test_latched_nip44_ciphertext_body_is_accepted() {
|
||||
let sender = nostr::Keys::generate();
|
||||
let recipient = nostr::Keys::generate();
|
||||
let ciphertext = nostr::nips::nip44::encrypt(
|
||||
sender.secret_key(),
|
||||
&recipient.public_key(),
|
||||
"encrypted approval note",
|
||||
nostr::nips::nip44::Version::V2,
|
||||
)
|
||||
.expect("encrypt");
|
||||
assert!(latched_body_disposition(&ciphertext).is_ok());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -413,12 +413,14 @@ pub(crate) fn requires_h_channel_scope(kind: u32) -> bool {
|
||||
/// silently store plaintext — the exact leak the latch exists to prevent.
|
||||
///
|
||||
/// This is the content-bearing half of the relay's channel-scoped acceptance
|
||||
/// surface (`requires_h_channel_scope`). The two must stay in lockstep: any kind
|
||||
/// the relay accepts as channel-scoped is either gated here (free-text body) or
|
||||
/// listed as bodyless in the `e2e_drift_guard` test. That guard derives from
|
||||
/// `requires_h_channel_scope` and fails if a new channel-scoped kind is added
|
||||
/// surface. The `e2e_drift_guard` test derives the full surface directly —
|
||||
/// `required_scope_for_kind(..).is_ok() && !is_global_only_kind(..)` — and
|
||||
/// asserts every channel-scoped kind lands in exactly one bucket: gated here,
|
||||
/// bodyless, or (for kinds that short-circuit before the 15c gate) enforced at
|
||||
/// the command path. The guard fails if a new channel-scoped kind is added
|
||||
/// without classification, so this list cannot silently drift behind the
|
||||
/// acceptance surface (the bug that left 40003-40007 ungated across two passes).
|
||||
/// acceptance surface (the bug that left 40003-40007, then the command kinds,
|
||||
/// ungated across earlier passes).
|
||||
///
|
||||
/// 40004 (pinned) and 40005 (bookmarked) are gated despite having no SDK builder
|
||||
/// or relay-side schema: they are named "a stream message that has been
|
||||
@@ -2339,11 +2341,19 @@ mod tests {
|
||||
/// - NIP-29 admin (put/remove user, edit metadata, delete event/group,
|
||||
/// leave request): membership/admin commands, empty or structured content;
|
||||
/// - huddle lifecycle (started/joined/left/ended/guidelines): structured
|
||||
/// session state, no message body.
|
||||
/// session state, no message body;
|
||||
/// - deletion (kind:5): references targets by `e` tag; any reason text is
|
||||
/// not channel-display content, and gating would break deletes in a DM;
|
||||
/// - reaction (kind:7): emoji or "+"/"-"; resolves its channel via
|
||||
/// `derive_reaction_channel`, and gating would break DM reactions;
|
||||
/// - gift wrap (kind:1059): NIP-59 sealed envelope, already ciphertext;
|
||||
/// - NIP-29 create-group (9007) / join-request (9021): channel-lifecycle
|
||||
/// commands, structured/empty content, no message body.
|
||||
///
|
||||
/// This list is the test's accounting of the bodyless half of the
|
||||
/// channel-scoped surface; `e2e_drift_guard` asserts the two halves together
|
||||
/// cover every channel-scoped kind, so a new kind can't slip in unclassified.
|
||||
/// channel-scoped surface; `e2e_drift_guard` asserts every channel-scoped
|
||||
/// kind lands in exactly one classification bucket, so a new kind can't slip
|
||||
/// in unclassified.
|
||||
const BODYLESS_CHANNEL_SCOPED_KINDS: &[u32] = &[
|
||||
KIND_FORUM_VOTE,
|
||||
KIND_NIP29_PUT_USER,
|
||||
@@ -2357,30 +2367,94 @@ mod tests {
|
||||
KIND_HUDDLE_PARTICIPANT_LEFT,
|
||||
KIND_HUDDLE_ENDED,
|
||||
KIND_HUDDLE_GUIDELINES,
|
||||
KIND_DELETION,
|
||||
KIND_REACTION,
|
||||
KIND_GIFT_WRAP,
|
||||
KIND_NIP29_CREATE_GROUP,
|
||||
KIND_NIP29_JOIN_REQUEST,
|
||||
];
|
||||
|
||||
/// Command kinds whose body carries free text the relay reads in plaintext
|
||||
/// (workflow YAML, trigger JSON inputs, approval note). They short-circuit at
|
||||
/// the `is_command_kind` branch in `handle_event` BEFORE the 15c gate, so
|
||||
/// they CANNOT be E2E-gated there. The latched-channel boundary is enforced
|
||||
/// for them at the command path instead (`enforce_latched_body` /
|
||||
/// `enforce_latched_approval_note` in `command_executor`), via the body-shape
|
||||
/// rule "empty or NIP-44, else reject" — not a per-kind list, so it cannot
|
||||
/// drift.
|
||||
const COMMAND_CONTENT_BEARING_KINDS: &[u32] = &[
|
||||
KIND_WORKFLOW_DEF,
|
||||
KIND_WORKFLOW_TRIGGER,
|
||||
KIND_APPROVAL_GRANT,
|
||||
KIND_APPROVAL_DENY,
|
||||
];
|
||||
|
||||
/// Command kinds that carry no free-text body — DM management commands keyed
|
||||
/// by tags (member pubkey, channel ref), structured/empty content. They also
|
||||
/// short-circuit before the 15c gate; the command-path body-shape rule admits
|
||||
/// their empty content unchanged, so they are never gated.
|
||||
const COMMAND_EMPTY_BODY_KINDS: &[u32] = &[KIND_DM_OPEN, KIND_DM_ADD_MEMBER, KIND_DM_HIDE];
|
||||
|
||||
#[test]
|
||||
fn e2e_drift_guard_classifies_every_channel_scoped_kind() {
|
||||
// Structural fix for the kind-set-drift bug class: the E2E gate
|
||||
// (`is_e2e_enforced_content_kind`) is a hand-written list that ran
|
||||
// parallel to the relay's actual channel-scoped acceptance surface
|
||||
// (`requires_h_channel_scope`) and drifted from it, leaving content kinds
|
||||
// ungated. This guard derives directly from the acceptance surface: every
|
||||
// kind the relay accepts as channel-scoped MUST be classified as either
|
||||
// E2E-gated (carries a free-text body) or explicitly bodyless. Adding a
|
||||
// new arm to `requires_h_channel_scope` without classifying it here fails
|
||||
// this test — so the gate can't silently drift behind the surface again.
|
||||
// parallel to a NARROWER proxy (`requires_h_channel_scope`) than the
|
||||
// relay's actual channel-scoped acceptance surface, and drifted from it —
|
||||
// leaving command content kinds (WORKFLOW_DEF, APPROVAL_*) ungated. This
|
||||
// guard derives directly from the REAL surface: a kind that resolves a
|
||||
// `channel_id` and is accepted (`required_scope_for_kind(..).is_ok()`) and
|
||||
// is not global-only. Every such kind MUST land in exactly one bucket:
|
||||
// 1. 15c-gated (free-text body, reaches the 15c gate);
|
||||
// 2. bodyless (no free-text content);
|
||||
// 3. command content-bearing (free-text body, gated at the command path);
|
||||
// 4. command empty-body (structured command, no body).
|
||||
// Adding a new accepted channel-scoped kind without classifying it here
|
||||
// fails this test — so the gate can't silently drift behind the surface.
|
||||
let dummy = make_dummy_event();
|
||||
for kind in 0u32..=50_000 {
|
||||
if !requires_h_channel_scope(kind) {
|
||||
let channel_scoped =
|
||||
required_scope_for_kind(kind, &dummy).is_ok() && !is_global_only_kind(kind);
|
||||
if !channel_scoped {
|
||||
continue;
|
||||
}
|
||||
let gated = is_e2e_enforced_content_kind(kind);
|
||||
let bodyless = BODYLESS_CHANNEL_SCOPED_KINDS.contains(&kind);
|
||||
let buckets = [
|
||||
is_e2e_enforced_content_kind(kind),
|
||||
BODYLESS_CHANNEL_SCOPED_KINDS.contains(&kind),
|
||||
COMMAND_CONTENT_BEARING_KINDS.contains(&kind),
|
||||
COMMAND_EMPTY_BODY_KINDS.contains(&kind),
|
||||
];
|
||||
let matched = buckets.iter().filter(|b| **b).count();
|
||||
assert_eq!(
|
||||
matched, 1,
|
||||
"channel-scoped kind {kind} must land in EXACTLY ONE classification \
|
||||
bucket [15c-gated, bodyless, command-content-bearing, \
|
||||
command-empty-body] — matched {matched}: {buckets:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e2e_drift_guard_command_kinds_partitioned() {
|
||||
// The 7 command kinds partition into content-bearing (latch-enforced at
|
||||
// the command path) XOR empty-body — no overlap, full coverage. This
|
||||
// pins the command-path side of the surface independently of the 15c
|
||||
// gate, which command kinds never reach.
|
||||
for kind in [
|
||||
KIND_WORKFLOW_DEF,
|
||||
KIND_WORKFLOW_TRIGGER,
|
||||
KIND_APPROVAL_GRANT,
|
||||
KIND_APPROVAL_DENY,
|
||||
KIND_DM_OPEN,
|
||||
KIND_DM_ADD_MEMBER,
|
||||
KIND_DM_HIDE,
|
||||
] {
|
||||
let content = COMMAND_CONTENT_BEARING_KINDS.contains(&kind);
|
||||
let empty = COMMAND_EMPTY_BODY_KINDS.contains(&kind);
|
||||
assert!(
|
||||
gated ^ bodyless,
|
||||
"channel-scoped kind {kind} is unclassified: it must be either \
|
||||
E2E-gated (free-text body) or in BODYLESS_CHANNEL_SCOPED_KINDS, \
|
||||
and exactly one of the two — got gated={gated}, bodyless={bodyless}"
|
||||
content ^ empty,
|
||||
"command kind {kind} must be exactly one of content-bearing or \
|
||||
empty-body — got content={content}, empty={empty}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user