createSpace requires `policy` and `appAccess` #631

Closed ngerakines.me
targeting main
Closed opened by and assigned to ngerakines.me
targeting main

com.atproto.simplespace.createSpace typed both fields as optional where its lexicon marks them required. A caller omitting them got 200 and a silently defaulted space.

That is how a shipped client (bulleted.app) sent {"type": "app.bulleted.space", "skey": "self"} in production for months: invalid against the schema, accepted by this server, and configured exactly as its owner intended, because the defaults applied were the ones they wanted. There was no wrong space to notice, no error to search for, and no host that disagreed — it was found by a person comparing the request against the lexicon.

The 0.15.0-rc.3 changelog deferred this decision. This takes it.

The decision #

Require the information, accept either spelling. A request must carry a policy and an app-access value somewhere; only "carries neither, anywhere" is refused. Every compatible shape still works — the lexicon's top-level unions, the nested config object, mintPolicy inside it, and the bare knownValues policy string. Retiring the legacy spellings is a separate decision with its own deprecation window and is not taken here.

Refusal is 400 InvalidRequest, deliberately not UnsupportedPolicy / UnsupportedAppAccess: those name a well-formed union variant this host cannot enforce, and a caller acts on them by choosing a different variant — the wrong instruction for a caller that sent no field at all. The message names both fields, since the whole failure mode is a client that has no idea its body is invalid.

Why it drifted, stated where the check lives #

Proposal 0016 both gives the fields defaults (member-list / #open) and marks them required. Not contradictory — a default describes the resulting configuration, required describes the request — but close enough that "absent means default" reads as obviously correct while writing the handler. That reasoning is now in the docblock so the permissive reading does not get re-derived into a re-opened decision.

Scope #

  • SpaceConfig::missing_required_create_fields sits beside policy_field, so the mintPolicy alias counts as carrying a policy.
  • The check is one arm on the merged config value create_space already computes — no second parse.
  • updateSpace is unchanged. Its optionality is the point: a partial update, where an owner changing only appAccess must not restate the policy.
  • SpaceConfig::default() and from_create_input are unchanged. The requirement is a property of the HTTP input, not of the config model, which has ~20 in-crate callers that legitimately construct a default.

One wrinkle, stated rather than discovered #

createSpace is idempotent on re-create (300bdf2), so a legacy client replaying a create it already made used to converge on 200 {uri} and will now get 400. That is the same refusal its first call would get today, so it is consistent rather than idempotency breaking — but it turns a previously-safe retry into an error, and someone will report it as "idempotency broke".

Tests #

  • Neither field → 400, asserting the error name, not just the status, and that both field names appear in the message. This is the regression: the test that would have made the client's bug visible from this side.
  • One field alone → refused too. Half a config is what a hand-written client actually produces, and policy.is_none() && app_access.is_none() would pass it.
  • All three legacy spellings still succeed and produce the config they asked for — the compatibility path asserted rather than assumed.
  • updateSpace still accepts a single field, leaving the unnamed one alone.
  • an_unimplemented_config_variant_is_refused_by_name now sends both fields and overrides one, so it still gets UnsupportedPolicy / UnsupportedAppAccess rather than the new refusal.
  • 14 existing call sites given conformant bodies. The bare-string-policy pair in create_space_replay_does_not_reconfigure_the_space kept its legacy spelling and gained an appAccess, so it still asserts what it was written to assert.

Notes #

  • Top-level mintPolicy was never accepted by createSpace — CreateSpaceInput has no such field, so serde dropped it. The legacy spelling reaches policy_field only nested inside config, and that is what the test asserts. The rc.3 changelog's claim was true of updateSpace.
  • apply_writes_returns_one_typed_result_per_write fails on main too (error-atproto-space-repo-3 duplicate key in batch), verified by stashing. Unrelated and untouched.
  • Not done: vendoring the com.atproto.simplespace.* lexicon JSONs and validating against them. "The lexicon marks them required" is still a claim in a doc comment with nothing behind it in this repo, which is part of why the handler drifted. Worth its own change.

This pull request has no versions submitted yet.