Everything could be created and nothing removed. `link add` and `expose` upsert
on (owner, name, kind), so re-running either edited an entry in place, but a name
typed wrong was permanent and there was no delete anywhere — no route, no
command.
The worse half was sharing. create_resource never touched the shares table and
share_with was INSERT OR IGNORE, so the list only ever grew:
expose … --name Agent --share @bob shares {bob}
expose … --name Agent --share @carol shares {bob, carol}
The second command reads as "this is Carol's now". Bob kept access, can_view
honoured the stale row, and nothing could remove it — a permission that could be
granted and never taken back. set_shares replaces the list in one transaction, so
the share list a command names is the whole share list afterwards, and naming
nobody revokes.
DELETE /api/resources/{id} scopes the delete by owner in the statement rather
than checking first, so a request naming someone else's resource deletes nothing
instead of relying on a caller to have asked the right question. It answers 404
rather than 403 for the same reason /api/capability does. Shares go with it by ON
DELETE CASCADE, which only began working when foreign_keys moved out of SCHEMA
and onto every connection — before that the pragma was a no-op in every process
that had not created the database.
`devsite link remove <name>` and `devsite unexpose <name>` resolve the name to an
id client-side, because the server keys everything by id and an endpoint deleting
by name would need a rule for a link and a service sharing one. Listing first
also means "no such thing" can be answered against the names that do exist.
unexpose drops the service from the local daemon config too: a config still
listing it would re-register the name on the next expose.
Verified against a running instance: Bob loses access when Agent is re-shared to
Carol, a removed link leaves the profile, unexpose clears both sides, and a
delete attempted by another account 404s while the resource survives.
75 tests.- commit
- 713eb4a0b58494649a7b8c49ee9c232b741c253c
- parent
- dd3fd256a16ed1e03c909bbc84d944ae812ff1e9
- author
- Dami <dami.etoile@gmail.com>
| M | README.md | 17 | ++++- |
| M | crates/ | 13 | +++ |
| M | crates/ | 79 | +++++++++++++++++++++ |
| M | crates/ | 32 | +++++++- |
| M | crates/ | 113 | +++++++++++++++++++++++++++--- |
| M | docs/ | 33 | ++++++++- |
@@ -75,6 +75,15 @@ devsite theme set my-theme.css
7575 devsite daemon run7676 ```7777 78+Re-running `link add` or `expose` with the same name edits that entry in place, and79+the share list it names replaces whatever was there — `--share @carol` means the80+resource is Carol's, not Carol's as well. To take something down:81+82+```bash83+devsite link remove "Klot Ski"84+devsite unexpose Hermes85+```86+7887 ### The profile page7988 8089 Semantic HTML styled by [Pico CSS], set in Open Sans at 400 and 700. Both are@@ -124,7 +133,7 @@ First-time setup, DNS, certificates and backups are in
124133 | `crates/devsite-web` | wasm-bindgen seam over `devsite-client`. |125134 | `crates/devsite-daemon` | Iroh endpoint, capability verification, loopback proxy. |126135 | `crates/devsite-server` | Control plane: axum + SQLite, Shoo auth, capability issuance. |127-| `crates/devsite-cli` | `devsite` — login, profile, link, expose, daemon run. |136+| `crates/devsite-cli` | `devsite` — login, profile, link, expose, unexpose, theme, daemon run. |128137 | `crates/devsite-fixture` | Serves one HTML file on a port; stands in for a real service. |129138 | `web/` | The site. `web/vendor/` is Pico and Open Sans; `web/pkg/` is generated by `scripts/build-wasm.sh`. |130139 @@ -137,7 +146,7 @@ fast native tests running the exact code the browser executes.
137146 cargo test --workspace138147 ```139148 140-68 tests. The interesting ones are `crates/devsite-daemon/tests/authz.rs`, which drives149+75 tests. The interesting ones are `crates/devsite-daemon/tests/authz.rs`, which drives141150 real Iroh against a real local service and asserts that forged, expired, misaddressed,142151 rebound, replayed and unknown-resource capabilities are each refused — and that a denial143152 tells the caller nothing about which of those it was. `theme.rs` carries the other@@ -157,6 +166,10 @@ property outside the whitelist.
157166 - A profile theme is a whitelist of Pico variables with per-property value grammars, so158167 the alphabet of a stored value cannot contain `<`, `"` or `}`. User styling cannot159168 position, hide or overlay anything, and cannot make one profile impersonate another.169+- A share names the whole list, so `--share @carol` revokes whoever was named before rather170+ than adding to them, and sharing with nobody takes it back entirely.171+- Deletes are scoped by owner inside the statement, not by a check beforehand, so a request172+ naming someone else's resource removes nothing.160173 - Session tokens are stored only as SHA-256 hashes.161174 - `issue-session` is an operator command requiring shell access; there is no162175 network-reachable authentication bypass.@@ -56,6 +56,19 @@ impl ControlPlane {
5656 decode(response, path).await5757 }5858 59+ /// DELETE, where a 2xx with no body is the expected success.60+ pub async fn delete(&self, path: &str) -> Result<()> {61+ let response = self62+ .request(reqwest::Method::DELETE, path)63+ .send()64+ .await65+ .with_context(|| format!("DELETE {path}"))?;66+ if !response.status().is_success() {67+ bail!("{path} failed: {}", error_text(response).await);68+ }69+ Ok(())70+ }71+5972 /// PUT where a 2xx with no body is the expected success.6073 pub async fn put_empty<B: Serialize>(&self, path: &str, body: &B) -> Result<()> {6174 let response = self@@ -51,6 +51,11 @@ enum Command {
5151 5252 share: Vec<String>,5353 },54+ /// Stop exposing a local service and take it off your profile.55+ Unexpose {56+ /// The name it was exposed under.57+ name: String,58+ },5459 /// Profile appearance.5560 5661 Theme(ThemeCommand),@@ -77,6 +82,9 @@ enum LinkCommand {
7782 7883 public: bool,7984 },85+ /// Take a link off your profile. Re-running `add` with the same name edits86+ /// it in place; this is for when it should not be there at all.87+ Remove { name: String },8088 }8189 8290 @@ -130,6 +138,20 @@ struct CreateResourceResponse {
130138 resource_id: String,131139 }132140 141+142+struct ResourceListing {143+ resources: Vec<Resource>,144+}145+146+147+struct Resource {148+ resource_id: String,149+ name: String,150+ kind: String,151+ /// Set for links only; a service's origin never leaves the machine serving it.152+ url: Option<String>,153+}154+133155 134156 async fn main() -> Result<()> {135157 tracing_subscriber::fmt()@@ -151,6 +173,10 @@ async fn main() -> Result<()> {
151173 Command::Link(LinkCommand::Add { name, url, public }) => {152174 add_link(&paths, &cli.server, &name, &url, public).await153175 }176+ Command::Link(LinkCommand::Remove { name }) => {177+ remove_link(&paths, &cli.server, &name).await178+ }179+ Command::Unexpose { name } => unexpose(&paths, &cli.server, &name).await,154180 Command::Expose {155181 origin,156182 name,@@ -248,6 +274,59 @@ async fn add_link(paths: &Paths, server: &str, name: &str, url: &str, public: bo
248274 Ok(())249275 }250276 277+/// Delete one of your resources, found by the name you gave it.278+///279+/// Names are resolved here rather than server-side because the server keys280+/// everything by id, and an endpoint that deleted by name would have to invent a281+/// rule for a link and a service sharing one. Listing first also means the "no282+/// such thing" case is answered with the names that do exist.283+async fn remove_resource(paths: &Paths, server: &str, name: &str, kind: &str) -> Result<Resource> {284+ let config = load_authenticated(paths)?;285+ let api = ControlPlane::new(server, config.session_token.clone());286+287+ let listing: ResourceListing = api.get("/api/resources").await?;288+ let found = listing289+ .resources290+ .into_iter()291+ .find(|r| r.name == name && r.kind == kind);292+293+ let Some(resource) = found else {294+ bail!("you have no {kind} called `{name}`");295+ };296+ api.delete(&format!("/api/resources/{}", resource.resource_id))297+ .await?;298+ Ok(resource)299+}300+301+async fn remove_link(paths: &Paths, server: &str, name: &str) -> Result<()> {302+ let removed = remove_resource(paths, server, name, "link").await?;303+ println!("removed link {name} → {}", removed.url.unwrap_or_default());304+ Ok(())305+}306+307+/// Stop serving a local service and take it off the profile.308+///309+/// Both halves matter and in this order: the control plane forgets it, then this310+/// machine does. A daemon that kept serving a resource the control plane has311+/// deleted is harmless — no capability can be issued for it any more — but a312+/// config that still lists it would re-register the name on the next `expose`.313+async fn unexpose(paths: &Paths, server: &str, name: &str) -> Result<()> {314+ remove_resource(paths, server, name, "service").await?;315+316+ let mut config = paths.load_config()?;317+ let before = config.resources.len();318+ config.resources.retain(|r| r.name != name);319+ paths.save_config(&config)?;320+321+ println!("unexposed {name}");322+ if config.resources.len() == before {323+ println!(" (this machine was not serving it; removed from your profile)");324+ } else {325+ println!(" restart `devsite daemon run` to stop serving it");326+ }327+ Ok(())328+}329+251330 async fn expose(252331 paths: &Paths,253332 server: &str,@@ -209,6 +209,7 @@ pub fn router(state: Shared) -> Router {
209209 .route("/api/me", get(me))210210 .route("/api/profile", post(claim_handle))211211 .route("/api/resources", post(create_resource).get(list_resources))212+ .route("/api/resources/{id}", axum::routing::delete(delete_resource))212213 .route("/api/daemon", axum::routing::put(register_daemon))213214 .route("/api/theme", get(read_theme).put(write_theme))214215 .route("/api/theme/properties", get(theme_properties))@@ -345,7 +346,7 @@ async fn create_resource(
345346 _ => {}346347 }347348 348- let db = state.db.lock().unwrap();349+ let mut db = state.db.lock().unwrap();349350 350351 // Resolve every share target *before* creating anything. Creating first and failing351352 // partway leaves an orphaned resource on the profile that the owner's daemon knows@@ -365,15 +366,38 @@ async fn create_resource(
365366 .create_resource(owner, &body.name, kind, visibility, body.url.as_deref(), now_secs())366367 .map_err(ApiError::internal)?;367368 368- for viewer in viewers {369- db.share_with(resource, viewer).map_err(ApiError::internal)?;370- }369+ // The share list this request names is the whole share list afterwards.370+ // Re-running `expose --share @carol` means the resource is Carol's, not371+ // Carol's as well as whoever was named the last time it ran.372+ db.set_shares(resource, &viewers).map_err(ApiError::internal)?;371373 372374 Ok(Json(CreateResourceResponse {373375 resource_id: resource.to_string(),374376 }))375377 }376378 379+/// Remove a resource. Only its owner can, and only their own.380+///381+/// A resource that is not yours is 404 rather than 403, as everywhere else here:382+/// a 403 would confirm the id exists.383+async fn delete_resource(384+ State(state): State<Shared>,385+ headers: HeaderMap,386+ Path(id): Path<String>,387+) -> ApiResult<impl IntoResponse> {388+ let owner = require_account(&state, &headers)?;389+ let resource = ResourceId::from_str(&id).map_err(|_| ApiError::not_found())?;390+391+ let db = state.db.lock().unwrap();392+ if !db393+ .delete_resource(owner, resource)394+ .map_err(ApiError::internal)?395+ {396+ return Err(ApiError::not_found());397+ }398+ Ok(StatusCode::NO_CONTENT)399+}400+377401 async fn list_resources(378402 State(state): State<Shared>,379403 headers: HeaderMap,@@ -376,14 +376,45 @@ impl Db {
376376 rows.into_iter().collect()377377 }378378 379- pub fn share_with(&self, resource: ResourceId, viewer: AccountId) -> Result<()> {380- self.conn.execute(381- "INSERT OR IGNORE INTO shares (resource_id, viewer_id) VALUES (?1, ?2)",382- params![resource.to_string(), viewer.to_string()],379+ /// Set exactly who a resource is shared with, replacing whoever was there.380+ ///381+ /// Replacing rather than adding is the whole point. `devsite expose --share382+ /// @carol` reads as "this is Carol's now", and an accumulating list would383+ /// leave everyone previously named still holding access — a permission that384+ /// could be granted and never taken back. Sharing with nobody revokes.385+ pub fn set_shares(&mut self, resource: ResourceId, viewers: &[AccountId]) -> Result<()> {386+ let tx = self.conn.transaction()?;387+ tx.execute(388+ "DELETE FROM shares WHERE resource_id = ?1",389+ params![resource.to_string()],383390 )?;391+ for viewer in viewers {392+ tx.execute(393+ "INSERT OR IGNORE INTO shares (resource_id, viewer_id) VALUES (?1, ?2)",394+ params![resource.to_string(), viewer.to_string()],395+ )?;396+ }397+ tx.commit()?;384398 Ok(())385399 }386400 401+ /// Delete a resource, and with it every share of it.402+ ///403+ /// Scoped by owner in the statement itself rather than checked beforehand,404+ /// so a request naming someone else's resource deletes nothing rather than405+ /// relying on a caller to have asked the right question first. The shares go406+ /// by `ON DELETE CASCADE`, which only works because `foreign_keys` is now set407+ /// on every connection.408+ ///409+ /// Returns whether anything was deleted.410+ pub fn delete_resource(&self, owner: AccountId, resource: ResourceId) -> Result<bool> {411+ let rows = self.conn.execute(412+ "DELETE FROM resources WHERE id = ?1 AND owner_id = ?2",413+ params![resource.to_string(), owner.to_string()],414+ )?;415+ Ok(rows > 0)416+ }417+387418 pub fn shared_with(&self, resource: ResourceId) -> Result<Vec<AccountId>> {388419 let mut stmt = self389420 .conn@@ -497,16 +528,77 @@ mod tests {
497528 assert_eq!(first.id, second.id, "a returning user must not get a new account");498529 }499530 531+ 532+ fn re_sharing_replaces_the_list_rather_than_adding_to_it() {533+ // The bug this exists to prevent: `expose --share @carol` reading as534+ // "and Carol too". A share that can be granted and never revoked is not535+ // a share, it is a one-way door.536+ let (mut db, alice, bob) = seeded();537+ let carol = db.upsert_account("ps_carol", 0).unwrap();538+ db.set_handle(carol.id, "carol").unwrap();539+540+ let agent = db541+ .create_resource(alice.id, "Agent", ResourceKind::Service, Visibility::Shared, None, 0)542+ .unwrap();543+544+ db.set_shares(agent, &[bob.id]).unwrap();545+ assert_eq!(db.shared_with(agent).unwrap(), vec![bob.id]);546+547+ db.set_shares(agent, &[carol.id]).unwrap();548+ assert_eq!(549+ db.shared_with(agent).unwrap(),550+ vec![carol.id],551+ "Bob should have lost access, not kept it alongside Carol"552+ );553+554+ // Sharing with nobody is how you take it back entirely.555+ db.set_shares(agent, &[]).unwrap();556+ assert!(db.shared_with(agent).unwrap().is_empty());557+ assert!(db.resources_shared_with(carol.id).unwrap().is_empty());558+ }559+560+ 561+ fn deleting_a_resource_takes_its_shares_with_it() {562+ let (mut db, alice, bob) = seeded();563+ let agent = db564+ .create_resource(alice.id, "Agent", ResourceKind::Service, Visibility::Shared, None, 0)565+ .unwrap();566+ db.set_shares(agent, &[bob.id]).unwrap();567+568+ assert!(db.delete_resource(alice.id, agent).unwrap());569+ assert!(db.resource(agent).unwrap().is_none());570+ assert!(571+ db.shared_with(agent).unwrap().is_empty(),572+ "the share should have cascaded away with the resource"573+ );574+ assert!(db.resources_shared_with(bob.id).unwrap().is_empty());575+ // Deleting it twice is not an error, it is just nothing.576+ assert!(!db.delete_resource(alice.id, agent).unwrap());577+ }578+579+ 580+ fn you_can_only_delete_your_own_resources() {581+ let (mut db, alice, bob) = seeded();582+ let hermes = db583+ .create_resource(alice.id, "Hermes", ResourceKind::Service, Visibility::Shared, None, 0)584+ .unwrap();585+ // Shared with Bob, so he can see it — which must not extend to removing it.586+ db.set_shares(hermes, &[bob.id]).unwrap();587+588+ assert!(!db.delete_resource(bob.id, hermes).unwrap());589+ assert!(db.resource(hermes).unwrap().is_some());590+ }591+500592 501593 fn shares_are_visible_to_the_named_viewer_only() {502- let (db, alice, bob) = seeded();594+ let (mut db, alice, bob) = seeded();503595 let agent = db504596 .create_resource(alice.id, "Agent", ResourceKind::Service, Visibility::Shared, None, 0)505597 .unwrap();506598 let hermes = db507599 .create_resource(alice.id, "Hermes", ResourceKind::Service, Visibility::Private, None, 0)508600 .unwrap();509- db.share_with(agent, bob.id).unwrap();601+ db.set_shares(agent, &[bob.id]).unwrap();510602 511603 let bobs = db.resources_shared_with(bob.id).unwrap();512604 assert_eq!(bobs.len(), 1);@@ -519,12 +611,11 @@ mod tests {
519611 520612 521613 fn owners_do_not_see_their_own_resources_as_shared_with_them() {522- let (db, alice, bob) = seeded();614+ let (mut db, alice, bob) = seeded();523615 let agent = db524616 .create_resource(alice.id, "Agent", ResourceKind::Service, Visibility::Shared, None, 0)525617 .unwrap();526- db.share_with(agent, bob.id).unwrap();527- db.share_with(agent, alice.id).unwrap();618+ db.set_shares(agent, &[bob.id, alice.id]).unwrap();528619 assert!(529620 db.resources_shared_with(alice.id).unwrap().is_empty(),530621 "own resources belong in the profile list, not `shared with me`"@@ -639,11 +730,11 @@ mod tests {
639730 // Not a hypothetical: the pragma lived in SCHEMA, which migrations skip640731 // for an existing database, so every process after the first ran without641732 // it. A share pointing at a deleted resource would have been left behind.642- let (db, alice, bob) = seeded();733+ let (mut db, alice, bob) = seeded();643734 let agent = db644735 .create_resource(alice.id, "Agent", ResourceKind::Service, Visibility::Shared, None, 0)645736 .unwrap();646- db.share_with(agent, bob.id).unwrap();737+ db.set_shares(agent, &[bob.id]).unwrap();647738 648739 db.conn649740 .execute("DELETE FROM resources WHERE id = ?1", params![agent.to_string()])@@ -180,7 +180,38 @@ than being greyed out in advance.
180180 Verified in a browser against a real daemon: reached with only an endpoint id and no relay181181 url anywhere in the response, then killed the daemon and got the timeout and its message.182182 183+## M9 — Taking things back ✅184+185+Everything could be created and nothing could be removed. `link add` and `expose` upsert on186+`(owner, name, kind)`, so re-running either edited an entry in place — but a name typed187+wrong was permanent, and there was no delete anywhere: no route, no command.188+189+The worse half was sharing. `create_resource` never touched the `shares` table and190+`share_with` was `INSERT OR IGNORE`, so the list only ever grew:191+192+```193+expose … --name Agent --share @bob shares {bob}194+expose … --name Agent --share @carol shares {bob, carol}195+```196+197+The second command reads as "this is Carol's now". Bob kept access, `can_view` honoured the198+stale row, and there was no way to remove it — a permission that could be granted and never199+taken back. `set_shares` now replaces the list in a transaction, so naming nobody revokes.200+201+`DELETE /api/resources/{id}` scopes the delete by owner in the statement itself rather than202+checking first, so a request naming someone else's resource deletes nothing rather than203+depending on a caller having asked the right question. Shares go with it by `ON DELETE204+CASCADE` — which only started working when `foreign_keys` moved out of `SCHEMA` and into205+every connection, in M8's Fly work. That pragma had been a no-op in every process that did206+not create the database.207+208+`devsite link remove <name>` and `devsite unexpose <name>` resolve the name to an id209+client-side and delete by id. `unexpose` also drops the service from the local daemon210+config, because a config that still lists it would re-register the name on the next211+`expose`.212+183213 ## Next184214 185215 Surfaces that still have none: managing exposures from the website, and seeing who a186-service is shared with.216+service is shared with. Renaming, too — remove and re-add is the only way, and for a217+service that means a new resource id.