mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): validate full NIP-98 probe invariant before authorizing
The probe accepted every structurally deserializable 2xx as Nip98Authorized and forwarded whatever role/source strings arrived, without requiring status == "ok", authMode == "nip98", a recognized non-null role/source, canAct == true, or canStaff == (role == operator). A token-mode, error-status, or role/capability-mismatched body reaching the authenticated retry authorized the console and could expose Staffing against a relay that denied it — a fail-open network-boundary classification. The unauthenticated 200 branch matched authMode alone. Add typed ProbeRole/ProbeSource enums (closed vocabularies) and two validators on ProbeWire: authorized_principal returns the typed principal only when the complete nip98 invariant holds; is_coherent_disabled gates the disabled branch on the full disabled-mode shape. Both call sites fail closed to NotAdminApi otherwise. parse_probe stays pure structural deserialization with Serde's unknown-field tolerance for forward compatibility. Route FeedbackStatusControl mutation failures through toast.error(adminErrorMessage(e)) like the other three handlers, so a relay error envelope no longer renders as raw JSON inline. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -172,14 +172,16 @@ async fn admin_probe_inner(
|
||||
return Ok(AdminProbeResult::NetworkOrIntercepted);
|
||||
}
|
||||
|
||||
// Step 3: success without auth → disabled mode (if body is a valid probe).
|
||||
// Step 3: success without auth → disabled mode (only when the body is a
|
||||
// coherent disabled-mode probe: status ok, authMode disabled, no
|
||||
// principal, no capabilities). A 200 in any other shape or mode is a
|
||||
// contract violation (token/nip98 must 401 an unauthenticated caller);
|
||||
// classify defensively.
|
||||
if resp.status().is_success() {
|
||||
let content_type = response_content_type(&resp);
|
||||
let bytes = read_bounded(resp, PROBE_JSON_CAP).await?;
|
||||
return Ok(match parse_probe(&content_type, &bytes) {
|
||||
Some(p) if p.auth_mode == "disabled" => AdminProbeResult::Disabled,
|
||||
// A 200 in any other mode is a contract violation (token/nip98
|
||||
// must 401 an unauthenticated caller); classify defensively.
|
||||
Some(p) if p.is_coherent_disabled() => AdminProbeResult::Disabled,
|
||||
_ => AdminProbeResult::NotAdminApi,
|
||||
});
|
||||
}
|
||||
@@ -223,11 +225,16 @@ async fn admin_probe_inner(
|
||||
let content_type = response_content_type(&auth_resp);
|
||||
let bytes = read_bounded(auth_resp, PROBE_JSON_CAP).await?;
|
||||
return Ok(match parse_probe(&content_type, &bytes) {
|
||||
// Relay resolves role/source for the authenticated
|
||||
// principal; carry them through for the staffing tab.
|
||||
Some(p) => AdminProbeResult::Nip98Authorized {
|
||||
role: p.role,
|
||||
source: p.source,
|
||||
// Trust the 2xx only when the full NIP-98 invariant holds;
|
||||
// carry the relay-resolved role/source for the staffing tab.
|
||||
Some(p) => match p.authorized_principal() {
|
||||
Some((role, source)) => AdminProbeResult::Nip98Authorized {
|
||||
role: Some(role.as_str().to_string()),
|
||||
source: Some(source.as_str().to_string()),
|
||||
},
|
||||
// Structurally a probe body but not a coherent
|
||||
// authorized NIP-98 response: fail closed.
|
||||
None => AdminProbeResult::NotAdminApi,
|
||||
},
|
||||
// 2xx but not a probe shape: endpoint exists but isn't
|
||||
// the admin API.
|
||||
@@ -296,23 +303,109 @@ async fn read_bounded(resp: reqwest::Response, cap: u64) -> Result<Vec<u8>, Stri
|
||||
/// The relay's `/probe` response contract (`ProbeResponse` in the relay's
|
||||
/// `api/admin/mod.rs`, serialised `rename_all = "camelCase"`).
|
||||
///
|
||||
/// Only the fields the desktop consumes are typed. `role`/`source` are
|
||||
/// present (non-null) only in nip98 mode with a resolved principal; they are
|
||||
/// `null` in token/disabled modes.
|
||||
/// All six fields are required and typed; deserialisation rejects a body that
|
||||
/// omits any field or carries a wrong-typed one, so an unrelated JSON endpoint
|
||||
/// cannot be mistaken for the admin API. Unknown fields are tolerated for
|
||||
/// forward compatibility. Structural validity alone does NOT authorize: a
|
||||
/// deserialised `ProbeWire` still has to pass [`ProbeWire::authorized_principal`]
|
||||
/// or [`ProbeWire::is_coherent_disabled`] before its state is trusted.
|
||||
#[derive(serde::Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct ProbeWire {
|
||||
#[allow(dead_code)]
|
||||
status: String,
|
||||
auth_mode: String,
|
||||
role: Option<String>,
|
||||
source: Option<String>,
|
||||
#[allow(dead_code)]
|
||||
can_act: bool,
|
||||
#[allow(dead_code)]
|
||||
can_staff: bool,
|
||||
}
|
||||
|
||||
/// The relay's resolved principal role. Closed vocabulary — an unknown string
|
||||
/// fails to parse and denies authorization.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ProbeRole {
|
||||
Operator,
|
||||
Moderator,
|
||||
}
|
||||
|
||||
impl ProbeRole {
|
||||
fn parse(s: &str) -> Option<Self> {
|
||||
match s {
|
||||
"operator" => Some(Self::Operator),
|
||||
"moderator" => Some(Self::Moderator),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Operator => "operator",
|
||||
Self::Moderator => "moderator",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// How the relay established the principal's role. Closed vocabulary.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ProbeSource {
|
||||
Config,
|
||||
OwnerFallback,
|
||||
Db,
|
||||
}
|
||||
|
||||
impl ProbeSource {
|
||||
fn parse(s: &str) -> Option<Self> {
|
||||
match s {
|
||||
"config" => Some(Self::Config),
|
||||
"owner_fallback" => Some(Self::OwnerFallback),
|
||||
"db" => Some(Self::Db),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Config => "config",
|
||||
Self::OwnerFallback => "owner_fallback",
|
||||
Self::Db => "db",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ProbeWire {
|
||||
/// Validate the complete NIP-98 authorization invariant and return the
|
||||
/// typed principal only when every field is coherent with the relay
|
||||
/// contract: `status == "ok"`, `authMode == "nip98"`, a recognised
|
||||
/// non-null `role`/`source`, `canAct == true`, and
|
||||
/// `canStaff == (role == operator)`. Any deviation yields `None`, so the
|
||||
/// caller classifies the response `NotAdminApi` rather than trusting a
|
||||
/// fail-open 2xx.
|
||||
fn authorized_principal(&self) -> Option<(ProbeRole, ProbeSource)> {
|
||||
if self.status != "ok" || self.auth_mode != "nip98" {
|
||||
return None;
|
||||
}
|
||||
let role = ProbeRole::parse(self.role.as_deref()?)?;
|
||||
let source = ProbeSource::parse(self.source.as_deref()?)?;
|
||||
if !self.can_act || self.can_staff != (role == ProbeRole::Operator) {
|
||||
return None;
|
||||
}
|
||||
Some((role, source))
|
||||
}
|
||||
|
||||
/// Validate the disabled-mode invariant for an unauthenticated 200:
|
||||
/// `status == "ok"`, `authMode == "disabled"`, no principal, no
|
||||
/// capabilities. Any deviation is a contract violation (token/nip98 must
|
||||
/// 401 an unauthenticated caller).
|
||||
fn is_coherent_disabled(&self) -> bool {
|
||||
self.status == "ok"
|
||||
&& self.auth_mode == "disabled"
|
||||
&& self.role.is_none()
|
||||
&& self.source.is_none()
|
||||
&& !self.can_act
|
||||
&& !self.can_staff
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse a `/probe` response body into a [`ProbeWire`], returning `None` when
|
||||
/// the Content-Type is not JSON or the body does not match the probe contract.
|
||||
///
|
||||
|
||||
@@ -153,6 +153,91 @@ fn parse_probe_rejects_non_object() {
|
||||
assert!(parse_probe("application/json", b"not json").is_none());
|
||||
}
|
||||
|
||||
// ── authorized_principal / is_coherent_disabled invariants ────────────────
|
||||
//
|
||||
// Structural deserialisation (parse_probe) is necessary but not sufficient:
|
||||
// a 2xx body must also satisfy the full relay contract before its state is
|
||||
// trusted. These pin every branch of that invariant.
|
||||
|
||||
/// Parse a body known to be structurally valid, then validate it.
|
||||
fn authorized(body: &str) -> Option<(ProbeRole, ProbeSource)> {
|
||||
parse_probe("application/json", body.as_bytes())
|
||||
.expect("structurally valid probe")
|
||||
.authorized_principal()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorized_principal_accepts_every_coherent_shape() {
|
||||
// Operator (canStaff true) and moderator (canStaff false) across all three
|
||||
// recognised sources — each must yield the typed principal.
|
||||
let cases: &[(String, ProbeRole, ProbeSource)] = &[
|
||||
(
|
||||
probe_json("nip98", r#""operator""#, r#""config""#, true, true),
|
||||
ProbeRole::Operator,
|
||||
ProbeSource::Config,
|
||||
),
|
||||
(
|
||||
probe_json("nip98", r#""operator""#, r#""owner_fallback""#, true, true),
|
||||
ProbeRole::Operator,
|
||||
ProbeSource::OwnerFallback,
|
||||
),
|
||||
(
|
||||
probe_json("nip98", r#""moderator""#, r#""db""#, true, false),
|
||||
ProbeRole::Moderator,
|
||||
ProbeSource::Db,
|
||||
),
|
||||
];
|
||||
for (body, role, source) in cases {
|
||||
assert_eq!(authorized(body), Some((*role, *source)));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn authorized_principal_rejects_every_incoherent_shape() {
|
||||
// Structurally valid probe bodies the relay never emits under nip98;
|
||||
// accepting any is fail-open. One invariant broken per row, top to bottom:
|
||||
// wrong status, non-nip98 authMode, missing role, unknown role, missing
|
||||
// source, unknown source, false canAct, operator lacking canStaff,
|
||||
// moderator carrying canStaff.
|
||||
let pj = probe_json;
|
||||
let cases = [
|
||||
pj("nip98", r#""operator""#, r#""config""#, true, true)
|
||||
.replace(r#""status":"ok""#, r#""status":"error""#),
|
||||
pj("token", "null", "null", false, false),
|
||||
pj("nip98", "null", r#""config""#, true, true),
|
||||
pj("nip98", r#""superuser""#, r#""config""#, true, true),
|
||||
pj("nip98", r#""operator""#, "null", true, true),
|
||||
pj("nip98", r#""operator""#, r#""ldap""#, true, true),
|
||||
pj("nip98", r#""operator""#, r#""config""#, false, true),
|
||||
pj("nip98", r#""operator""#, r#""config""#, true, false),
|
||||
pj("nip98", r#""moderator""#, r#""config""#, true, true),
|
||||
];
|
||||
for (i, body) in cases.iter().enumerate() {
|
||||
assert_eq!(authorized(body), None, "row {i} must be rejected");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_coherent_disabled_accepts_only_canonical_disabled() {
|
||||
// Canonical disabled authorizes; nip98 mode and any disabled body claiming
|
||||
// a role/source or a capability is incoherent and must be rejected.
|
||||
let disabled = probe_json("disabled", "null", "null", false, false);
|
||||
assert!(parse_probe("application/json", disabled.as_bytes())
|
||||
.unwrap()
|
||||
.is_coherent_disabled());
|
||||
|
||||
let incoherent = [
|
||||
probe_json("nip98", r#""operator""#, r#""config""#, true, true),
|
||||
probe_json("disabled", r#""operator""#, "null", false, false),
|
||||
probe_json("disabled", "null", "null", true, false),
|
||||
];
|
||||
for body in &incoherent {
|
||||
assert!(!parse_probe("application/json", body.as_bytes())
|
||||
.unwrap()
|
||||
.is_coherent_disabled());
|
||||
}
|
||||
}
|
||||
|
||||
// ── Storage core through production code ─────────────────────────────────
|
||||
//
|
||||
// All tests call `get_admin_origin_core` / `set_admin_origin_core` directly
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
import {
|
||||
type AsyncState,
|
||||
type AttachmentMeta,
|
||||
adminErrorMessage,
|
||||
CommunityGroupedList,
|
||||
DetailRow,
|
||||
ErrorMessage,
|
||||
@@ -331,20 +332,18 @@ function FeedbackStatusControl({
|
||||
onStatusChanged: (newStatus: AdminFeedbackStatus) => void;
|
||||
}) {
|
||||
const [isWorking, setIsWorking] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
const statuses: AdminFeedbackStatus[] = ["new", "reviewed", "archived"];
|
||||
|
||||
const handleStatusChange = async (newStatus: AdminFeedbackStatus) => {
|
||||
if (newStatus === currentStatus) return;
|
||||
setError(null);
|
||||
setIsWorking(true);
|
||||
try {
|
||||
await patchAdminFeedback(origin, feedbackId, newStatus);
|
||||
toast.success(`Feedback marked ${newStatus}`);
|
||||
onStatusChanged(newStatus);
|
||||
} catch (e) {
|
||||
setError(e instanceof Error ? e.message : String(e));
|
||||
toast.error(adminErrorMessage(e));
|
||||
} finally {
|
||||
setIsWorking(false);
|
||||
}
|
||||
@@ -375,7 +374,6 @@ function FeedbackStatusControl({
|
||||
</Button>
|
||||
))}
|
||||
</div>
|
||||
{error && <p className="text-xs text-destructive">{error}</p>}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3099,3 +3099,92 @@ test("feedback-list-refetches-on-back-after-mutation: changing status then navig
|
||||
|
||||
await unmount();
|
||||
});
|
||||
|
||||
test("resolve-rejection-surfaces-parsed-message: a rejected resolve toasts the relay message, not raw JSON", async () => {
|
||||
// A resolve mutation that the relay rejects (e.g. invalid_action_for_target)
|
||||
// must surface the envelope's human message via toast.error — never the raw
|
||||
// JSON envelope and never a success toast.
|
||||
//
|
||||
// Mutation evidence: replace `toast.error(adminErrorMessage(e))` in
|
||||
// handleSubmit with `toast.error(String(e))` → the raw-JSON assertion goes
|
||||
// red because the envelope leaks verbatim.
|
||||
|
||||
const origin = "https://admin.example.com";
|
||||
const pubkey = "f7".repeat(32);
|
||||
|
||||
const openItem = {
|
||||
id: "00000000-0000-0000-0000-0000000000f7",
|
||||
communityId: "comm-1",
|
||||
communityHost: "alpha.example.com",
|
||||
reportEventId: "aa",
|
||||
reporterPubkey: "bb",
|
||||
targetKind: "event",
|
||||
target: "cc",
|
||||
reportType: "spam",
|
||||
status: "open",
|
||||
createdAt: "2024-06-01T12:00:00Z",
|
||||
};
|
||||
const openDetail = {
|
||||
...openItem,
|
||||
channelId: "00000000-0000-0000-0000-0000000000ff",
|
||||
note: null,
|
||||
resolvedBy: null,
|
||||
resolvedAt: null,
|
||||
actionId: null,
|
||||
message: null,
|
||||
};
|
||||
|
||||
const humanMessage =
|
||||
"action kick requires the report to have an associated channel";
|
||||
// The native command rejects with `admin API error: {envelope}` — exactly the
|
||||
// shape adminErrorMessage strips down to the envelope's `message`.
|
||||
const rawError = `admin API error: {"error":{"code":"invalid_action_for_target","message":"${humanMessage}","requestId":"00000000-0000-0000-0000-0000000000e7"}}`;
|
||||
|
||||
setIpcHandler("admin_list_reports", () => Promise.resolve([openItem]));
|
||||
setIpcHandler("admin_get_report", () => Promise.resolve(openDetail));
|
||||
setIpcHandler("admin_list_feedback", () => Promise.resolve([]));
|
||||
setIpcHandler("admin_resolve_report", () =>
|
||||
Promise.reject(new Error(rawError)),
|
||||
);
|
||||
|
||||
const { container, doRender, unmount } = mountPanel({ origin, pubkey });
|
||||
await doRender();
|
||||
await settle(30);
|
||||
await openFirstReportDetail(container);
|
||||
await settle(20);
|
||||
|
||||
// Select the kick action, then submit — the relay rejects it.
|
||||
const kickBtn = container.querySelector("[data-testid='action-btn-kick']");
|
||||
assert.ok(kickBtn, "kick action must be present (channel is set)");
|
||||
await act(async () => {
|
||||
fireEvent.click(kickBtn);
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
});
|
||||
const submit = container.querySelector("[data-testid='resolve-submit-btn']");
|
||||
assert.ok(submit, "resolve submit button must appear after selecting kick");
|
||||
await act(async () => {
|
||||
fireEvent.click(submit);
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
});
|
||||
await settle(20);
|
||||
|
||||
// The parsed human message reaches toast.error.
|
||||
assert.ok(
|
||||
capturedErrorToasts.some((m) => m.includes(humanMessage)),
|
||||
`the parsed relay message must surface via toast.error; got: ${JSON.stringify(capturedErrorToasts)}`,
|
||||
);
|
||||
// The raw JSON envelope must NOT leak into any error toast.
|
||||
assert.ok(
|
||||
!capturedErrorToasts.some(
|
||||
(m) => m.includes('{"error"') || m.includes("admin API error:"),
|
||||
),
|
||||
`the raw JSON envelope must not appear in an error toast; got: ${JSON.stringify(capturedErrorToasts)}`,
|
||||
);
|
||||
// No success toast on a rejected resolve.
|
||||
assert.ok(
|
||||
!capturedToasts.some((m) => m.toLowerCase().includes("resolved")),
|
||||
`no success toast on a rejected resolve; got: ${JSON.stringify(capturedToasts)}`,
|
||||
);
|
||||
|
||||
await unmount();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user