Skip to content

[channel] Make Server::admins and other public mutable fields private #118

@intendednull

Description

@intendednull

Parent: #108

Problem

crates/channel/src/lib.rs:329-342:

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Server {
    pub id: ServerId,
    pub name: String,
    pub description: Option<String>,
    pub creator: EndpointId,
    pub admins: HashSet<EndpointId>,   // <-- public!
    pub created_at: DateTime<Utc>,

    channels: HashMap<ChannelId, Channel>,
    roles: HashMap<RoleId, Role>,
    members: HashMap<EndpointId, Member>,
    invites: HashMap<InviteId, Invite>,
    ...
}

admins is pub, so any code with a &mut Server can do server.admins.insert(me) and grant itself admin status, bypassing the event-sourced authority model entirely. The crate's own tests exercise this (see tests.rs:806). Same for id, name, description which should also be private.

The event-sourced model in willow-state enforces authority correctly — the problem is that willow-channel exposes a parallel API that skips those checks. This is also a source of reviewer confusion about which crate enforces what.

Fix

  1. Make admins, id, name, description private.
  2. Add crate-level accessor methods (fn admins(&self) -> &HashSet<EndpointId>, fn name(&self) -> &str, etc.).
  3. Add a pub(crate) setter set_admin(&mut self, peer: EndpointId, is_admin: bool) intended to be called only from the state materializer or test helpers, with a doc comment explaining the contract.
  4. Add a module-level doc at the top of crates/channel/src/lib.rs clarifying: "This crate defines data structures only. All authority checks happen in willow-state::materialize::apply_*. Direct mutation of a Server is not an enforcement boundary."

This ties into issue #TBD (authority spec doc).

Test

Add a test that uses only the public API and asserts it cannot add an admin without going through the state machine. If you find any caller that relied on .admins.insert(), that's a bug to fix in that caller.

Migration

Likely breaks ~a handful of callers in willow-client, willow-app, and tests. Grep for .admins. and .admins = across the workspace. Replace with the new accessor / setter or remove the manual mutation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions