edit and delete, on both screens

the screens could show a calendar and an address book and change neither. this
adds a pencil and a bin to every event and every card, with a confirm on the bin.

three things worth defending.

THE HREF IS THE ADDRESS, and the listing now carries it. it used to return only
the object's data, which is enough to show something and not enough to change
it. deriving `<uid>.ics` was the obvious shortcut and is wrong: a .ics filename
only conventionally matches the uid — radicale stores what the writing client
chose — so it works for everything officer wrote and misses everything a phone
did. proved immediately: this machine's own calendar holds
62970EC7-47B1-4214-858E-85C32C2476A5.ics, written by an iphone.

READ, PATCH, PUT — NEVER REBUILD. an event is a file with far more in it than
these screens show: alarms, attendees, RRULEs, every X- property. rebuilding it
from the fields on screen would silently drop all of that, and an edit that
quietly deletes half an event is worse than no editing. so a save reads the
original text, replaces only the lines it owns, and puts the whole file back.
verified on a real event: 22 lines before, 22 after, one changed.

THE EDITOR IS DELIBERATELY SMALL. title and place for an event, name/email/phone
for a card. times and recurrence are where an editor does damage, and a
half-editor that rewrites an RRULE it does not understand is the thing this
avoids. `setProperty` also escapes commas, semicolons and newlines, because a
raw comma silently splits an iCalendar TEXT value into parts.

the sidecar grew GET/PUT/DELETE on /_officer/item, above the `collection is
required` guard — they take an href rather than a collection, and sitting below
it they answered 400 forever. both check the href is inside this user's own tree
even though X-Remote-User already scopes radicale: an href from a request body
is attacker-shaped input either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
2026-08-27 10:25:38 +00:00
co-authored by Claude Opus 5
parent 0faf9183ad
commit 785e5f9eed
7 changed files with 396 additions and 10 deletions
+30 -8
View File
@@ -99,7 +99,11 @@ export async function listCollections(fetcher: Fetcher, userId: string): Promise
* One report rather than a listing plus a GET per item: a calendar with a thousand events would
* otherwise be a thousand round trips.
*/
async function readObjects(fetcher: Fetcher, path: string, kind: 'calendar' | 'addressbook'): Promise<string[]> {
async function readObjects(
fetcher: Fetcher,
path: string,
kind: 'calendar' | 'addressbook',
): Promise<{ href: string; data: string }[]> {
const dataTag = kind === 'calendar' ? 'calendar-data' : 'address-data';
const ns = kind === 'calendar' ? 'urn:ietf:params:xml:ns:caldav' : 'urn:ietf:params:xml:ns:carddav';
const queryTag = kind === 'calendar' ? 'calendar-query' : 'addressbook-query';
@@ -114,17 +118,35 @@ async function readObjects(fetcher: Fetcher, path: string, kind: 'calendar' | 'a
});
if (!res.ok && res.status !== 207) return [];
const xml = await res.text();
return between(xml, dataTag)
.map((chunk) => decodeEntities(chunk).trim())
.filter(Boolean);
/*
Paired with the HREF, because a UID is not an address.
Reading returned only the object's data, which is enough to SHOW something and not enough to change
it: DELETE and PUT both need the item's URL, and a `.ics` filename is only conventionally the UID —
Radicale stores what the client that wrote it chose. Deriving `<uid>.ics` works for everything Officer
wrote and fails for everything a phone did, which is the worst kind of nearly-right.
So each `<response>` is split first and the href taken from it, rather than pulling every
`calendar-data` out of the whole document and losing which response each came from.
*/
return between(xml, 'response')
.map((block) => {
const href = decodeEntities(first(block, 'href') ?? '').trim();
const data = decodeEntities(first(block, dataTag) ?? '').trim();
return { href, data };
})
.filter((o) => o.href && o.data);
}
export async function listEvents(fetcher: Fetcher, path: string): Promise<CalendarEvent[]> {
const blobs = await readObjects(fetcher, path, 'calendar');
return blobs.flatMap(parseEvents);
const objects = await readObjects(fetcher, path, 'calendar');
// One .ics can hold several VEVENTs (a recurring series with overrides), and they share its href —
// which is correct: deleting any of them deletes the file they live in.
return objects.flatMap((o) => parseEvents(o.data).map((e) => ({ ...e, href: o.href })));
}
export async function listContacts(fetcher: Fetcher, path: string): Promise<Contact[]> {
const blobs = await readObjects(fetcher, path, 'addressbook');
return blobs.flatMap(parseContacts);
const objects = await readObjects(fetcher, path, 'addressbook');
return objects.flatMap((o) => parseContacts(o.data).map((c) => ({ ...c, href: o.href })));
}
+4
View File
@@ -63,6 +63,8 @@ export function parseDate(prop: Prop): { iso: string; allDay: boolean } | null {
export type CalendarEvent = {
uid: string;
/** Where this event LIVES — the DAV href. Absent when parsed outside a listing. Needed to edit or delete it. */
href?: string;
summary: string;
description?: string;
location?: string;
@@ -140,6 +142,8 @@ export function parseEvents(ics: string): CalendarEvent[] {
export type Contact = {
uid: string;
/** Where this contact LIVES — the DAV href. Absent when parsed outside a listing. Needed to edit or delete it. */
href?: string;
fullName: string;
emails: string[];
phones: string[];
+63
View File
@@ -185,6 +185,53 @@ const server = Bun.serve({
return Response.json({ collections: await listCollections(fetcher, String(userId)) });
}
/*
One item's ORIGINAL text, for an editor that must not lose what it does not understand.
Rebuilding an .ics from the parsed fields would silently drop alarms, attendees, recurrence rules
and every X- property the file came with — an edit that quietly deletes half an event is worse
than no editing at all. So the editor reads the real thing, changes the lines it owns, and puts
the whole file back.
*/
if (url.pathname === '/_officer/item' && req.method === 'GET') {
const href = url.searchParams.get('href') ?? '';
if (!href.startsWith(`/dav/${userId}/`)) {
return Response.json({ error: 'href is required and must be your own' }, { status: 400 });
}
const res = await fetcher(href, { method: 'GET' });
if (!res.ok) return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 });
return Response.json({ href, data: await res.text() });
}
if (url.pathname === '/_officer/item' && (req.method === 'PUT' || req.method === 'DELETE')) {
const body = (await req.json().catch(() => ({}))) as { href?: string; data?: string };
const href = String(body.href ?? '');
if (!href.startsWith(`/dav/${userId}/`)) {
return Response.json({ error: 'href is required and must be your own' }, { status: 400 });
}
if (req.method === 'DELETE') {
const res = await fetcher(href, { method: 'DELETE' });
// 404 is success from the caller's point of view: the thing they asked to be gone is gone.
if (!res.ok && res.status !== 404) {
return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 });
}
return Response.json({ ok: true });
}
if (!body.data) return Response.json({ error: 'data is required' }, { status: 400 });
const isCard = href.endsWith('.vcf');
const res = await fetcher(href, {
method: 'PUT',
headers: {
'Content-Type': isCard ? 'text/vcard; charset=utf-8' : 'text/calendar; charset=utf-8',
},
body: body.data,
});
if (!res.ok) return Response.json({ error: `radicale answered ${res.status}` }, { status: 502 });
return Response.json({ ok: true, status: res.status });
}
const path = url.searchParams.get('collection');
// Confine reads to this user's own tree. The header already scopes Radicale, but a path from a
// query string is attacker-shaped input and should not be handed on unchecked.
@@ -198,6 +245,22 @@ const server = Bun.serve({
if (url.pathname === '/_officer/contacts') {
return Response.json({ contacts: await listContacts(fetcher, path) });
}
/*
Writing, from Officer's own screens.
── Why the href is required and never derived ──
A `.ics` filename only conventionally matches the UID. Radicale stores whatever the client that
wrote it chose, so `<uid>.ics` finds everything Officer wrote and misses everything a phone did.
The listing hands back the real href; these take it back unchanged.
── Why it is checked against this user's tree ──
`X-Remote-User` already scopes Radicale, and an href arriving in a request body is
attacker-shaped input either way. Two gates rather than one, for the same reason the reads above
check `collection`.
*/
}
return new Response('not found', { status: 404 });
+98 -1
View File
@@ -1,6 +1,8 @@
import type { CalendarEvent } from './shared';
import { CalendarDays, MapPin, Repeat } from 'lucide-react';
import { useState } from 'react';
import { CalendarDays, MapPin, Repeat, Pencil, Trash2, Check, X } from 'lucide-react';
import { useCalendarEvents, useSelectedCollection } from './useDavData';
import { useDavEdit, setProperty } from './useDavEdit';
import { useTranslation } from 'officerdev';
// Right panel of /calendar. Deliberately a grouped agenda list, not a month grid: the sidecar returns
@@ -30,6 +32,11 @@ export const CalendarView = () => {
const { t } = useTranslation();
const { selected, isLoading: loadingCollections } = useSelectedCollection('calendar');
const { events, isLoading, error } = useCalendarEvents(selected?.path ?? null);
// Keyed on the COLLECTION, so a save refreshes the list this event came from.
const edit = useDavEdit(['caldav', 'events', selected?.path ?? null]);
const [editing, setEditing] = useState<string | null>(null);
const [draft, setDraft] = useState({ summary: '', location: '' });
const [confirming, setConfirming] = useState<string | null>(null);
if (loadingCollections) return <Placeholder text="Loading calendars…" />;
if (!selected) return <Placeholder text="No calendars yet. Create one from a CalDAV client." />;
@@ -74,6 +81,54 @@ export const CalendarView = () => {
)}
</div>
<div className="mt-0.5 text-xs tabular-nums text-muted-foreground">{formatTime(ev)}</div>
{/* The editor is the title and the place, and nothing else. Those are the two fields a
person actually corrects; times and recurrence are where an editor does damage, and
a half-editor that silently rewrites an RRULE is worse than none. */}
{editing === ev.href && (
<form
className="mt-2 flex flex-col gap-2"
onSubmit={(e) => {
e.preventDefault();
if (!ev.href) return;
void edit
.save(ev.href, (raw) =>
setProperty(setProperty(raw, 'SUMMARY', draft.summary), 'LOCATION', draft.location || null),
)
.then(() => setEditing(null));
}}
>
<input
autoFocus
value={draft.summary}
onChange={(e) => setDraft((d) => ({ ...d, summary: e.target.value }))}
placeholder="Title"
className="rounded-md border bg-background px-2 py-1 text-sm"
/>
<input
value={draft.location}
onChange={(e) => setDraft((d) => ({ ...d, location: e.target.value }))}
placeholder="Where"
className="rounded-md border bg-background px-2 py-1 text-xs"
/>
<div className="flex items-center gap-2">
<button
type="submit"
disabled={edit.busy === ev.href}
className="flex items-center gap-1 rounded-md bg-primary/10 px-2 py-1 text-xs text-primary disabled:opacity-50"
>
<Check className="h-3 w-3" /> {edit.busy === ev.href ? 'Saving…' : 'Save'}
</button>
<button
type="button"
onClick={() => setEditing(null)}
className="rounded-md px-2 py-1 text-xs text-muted-foreground"
>
Cancel
</button>
</div>
</form>
)}
{ev.location && (
<div className="mt-1 flex items-center gap-1 text-xs text-muted-foreground">
<MapPin className="h-3 w-3 shrink-0" />
@@ -84,6 +139,48 @@ export const CalendarView = () => {
<p className="mt-1 whitespace-pre-wrap text-xs text-muted-foreground">{ev.description}</p>
)}
</div>
{/* Only for an event we can actually address. An item with no href came from somewhere
that did not give us one, and offering a delete that cannot work is worse than not
offering it. */}
{ev.href && editing !== ev.href && (
<div className="flex shrink-0 items-start gap-1">
{confirming === ev.href ? (
<>
<button
onClick={() => void edit.remove(ev.href!).then(() => setConfirming(null))}
disabled={edit.busy === ev.href}
title="Delete this event"
className="rounded p-1 text-destructive hover:bg-destructive/10 disabled:opacity-50"
>
<Check className="h-3.5 w-3.5" />
</button>
<button onClick={() => setConfirming(null)} className="rounded p-1 opacity-50 hover:opacity-100">
<X className="h-3.5 w-3.5" />
</button>
</>
) : (
<>
<button
onClick={() => {
setDraft({ summary: ev.summary ?? '', location: ev.location ?? '' });
setEditing(ev.href!);
}}
title="Edit"
className="rounded p-1 text-muted-foreground hover:text-foreground"
>
<Pencil className="h-3.5 w-3.5" />
</button>
<button
onClick={() => setConfirming(ev.href!)}
title="Delete"
className="rounded p-1 text-muted-foreground hover:text-destructive"
>
<Trash2 className="h-3.5 w-3.5" />
</button>
</>
)}
</div>
)}
</article>
))}
</section>
+107 -1
View File
@@ -1,5 +1,7 @@
import { Building2, Mail, Phone, User } from 'lucide-react';
import { useState } from 'react';
import { Building2, Mail, Phone, User, Pencil, Trash2, Check, X } from 'lucide-react';
import { useDavContacts, useSelectedCollection } from './useDavData';
import { useDavEdit, setProperty } from './useDavEdit';
import { useTranslation } from 'officerdev';
// Right panel of /contacts: the selected address book as cards, sorted by name.
@@ -20,6 +22,10 @@ export const ContactsView = () => {
const { t } = useTranslation();
const { selected, isLoading: loadingCollections } = useSelectedCollection('addressbook');
const { contacts, isLoading, error } = useDavContacts(selected?.path ?? null);
const edit = useDavEdit(['caldav', 'contacts', selected?.path ?? null]);
const [editing, setEditing] = useState<string | null>(null);
const [draft, setDraft] = useState({ fullName: '', email: '', phone: '' });
const [confirming, setConfirming] = useState<string | null>(null);
if (loadingCollections) return <Placeholder text="Loading address books…" />;
if (!selected) return <Placeholder text="No address books yet. Create one from a CardDAV client." />;
@@ -65,7 +71,107 @@ export const ContactsView = () => {
<span className="truncate">{phone}</span>
</a>
))}
{/* Name, one email, one phone — the three fields worth correcting from a list. A card can
hold several of each; editing rewrites the FIRST and leaves the rest untouched, which
is why `setProperty` replaces a line rather than rebuilding the card. */}
{editing === contact.href && (
<form
className="mt-2 flex flex-col gap-2"
onSubmit={(e) => {
e.preventDefault();
if (!contact.href) return;
void edit
.save(contact.href, (raw) => {
let out = setProperty(raw, 'FN', draft.fullName);
out = setProperty(out, 'EMAIL', draft.email || null);
return setProperty(out, 'TEL', draft.phone || null);
})
.then(() => setEditing(null));
}}
>
<input
autoFocus
value={draft.fullName}
onChange={(e) => setDraft((d) => ({ ...d, fullName: e.target.value }))}
placeholder="Name"
className="rounded-md border bg-background px-2 py-1 text-sm"
/>
<input
value={draft.email}
onChange={(e) => setDraft((d) => ({ ...d, email: e.target.value }))}
placeholder="Email"
className="rounded-md border bg-background px-2 py-1 text-xs"
/>
<input
value={draft.phone}
onChange={(e) => setDraft((d) => ({ ...d, phone: e.target.value }))}
placeholder="Phone"
className="rounded-md border bg-background px-2 py-1 text-xs tabular-nums"
/>
<div className="flex items-center gap-2">
<button
type="submit"
disabled={edit.busy === contact.href}
className="flex items-center gap-1 rounded-md bg-primary/10 px-2 py-1 text-xs text-primary disabled:opacity-50"
>
<Check className="h-3 w-3" /> {edit.busy === contact.href ? 'Saving…' : 'Save'}
</button>
<button
type="button"
onClick={() => setEditing(null)}
className="rounded-md px-2 py-1 text-xs text-muted-foreground"
>
Cancel
</button>
</div>
</form>
)}
</div>
{contact.href && editing !== contact.href && (
<div className="flex shrink-0 items-start gap-1">
{confirming === contact.href ? (
<>
<button
onClick={() => void edit.remove(contact.href!).then(() => setConfirming(null))}
disabled={edit.busy === contact.href}
title="Delete this contact"
className="rounded p-1 text-destructive hover:bg-destructive/10 disabled:opacity-50"
>
<Check className="h-3.5 w-3.5" />
</button>
<button onClick={() => setConfirming(null)} className="rounded p-1 opacity-50 hover:opacity-100">
<X className="h-3.5 w-3.5" />
</button>
</>
) : (
<>
<button
onClick={() => {
setDraft({
fullName: contact.fullName ?? '',
email: contact.emails[0] ?? '',
phone: contact.phones[0] ?? '',
});
setEditing(contact.href!);
}}
title="Edit"
className="rounded p-1 text-muted-foreground hover:text-foreground"
>
<Pencil className="h-3.5 w-3.5" />
</button>
<button
onClick={() => setConfirming(contact.href!)}
title="Delete"
className="rounded p-1 text-muted-foreground hover:text-destructive"
>
<Trash2 className="h-3.5 w-3.5" />
</button>
</>
)}
</div>
)}
</article>
))}
</div>
+4
View File
@@ -14,6 +14,8 @@ export type DavCollection = {
export type CalendarEvent = {
uid: string;
/** The DAV href — where this event lives. What edit and delete address; never derived from the uid. */
href?: string;
summary: string;
description?: string;
location?: string;
@@ -27,6 +29,8 @@ export type CalendarEvent = {
export type DavContact = {
uid: string;
/** The DAV href — where this contact lives. What edit and delete address; never derived from the uid. */
href?: string;
fullName: string;
emails: string[];
phones: string[];
+90
View File
@@ -0,0 +1,90 @@
import { useState } from 'react';
import { useQueryClient } from '@tanstack/react-query';
import { useClient } from 'hooks/useClient';
/**
* Editing and deleting one DAV object, without losing what this app does not understand.
*
* ── Read, patch, put — never rebuild ──
*
* An event or a card is a file with far more in it than these screens show: alarms, attendees,
* recurrence rules, categories, every `X-` property the client that wrote it invented. Rebuilding the
* file from the fields on screen would silently drop all of it, and an edit that quietly deletes half
* an event is worse than no editing at all.
*
* So a save reads the ORIGINAL text, replaces only the lines it owns, and puts the whole thing back.
* Verified on a real event: 22 lines before, 22 after, one changed.
*
* ── The href is the address, and it comes from the listing ──
*
* Not `<uid>.ics`. That is only conventionally the filename — Radicale stores what the writing client
* chose, so deriving it works for everything Officer wrote and misses everything a phone did. The
* listing returns the real href; this hands it straight back.
*/
/** Replace a property's line, or append it when the file has none. Handles `PROP:` and `PROP;PARAM:`. */
export function setProperty(raw: string, prop: string, value: string | null): string {
const lines = raw.split(/\r?\n/);
const at = lines.findIndex((l) => l === prop || l.startsWith(`${prop}:`) || l.startsWith(`${prop};`));
if (value === null) return at === -1 ? raw : [...lines.slice(0, at), ...lines.slice(at + 1)].join('\r\n');
// iCalendar escapes these in TEXT values; a raw comma or semicolon silently splits the value into
// parts and a newline ends the property early, which is how a description eats the rest of a file.
const escaped = value.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/([,;])/g, '\\$1');
if (at !== -1) {
// Keep any parameters the line already carried — `DTSTART;TZID=…` must not become `DTSTART:`.
const params = /^[A-Z-]+(;[^:]*)?:/.exec(lines[at]!)?.[1] ?? '';
return [...lines.slice(0, at), `${prop}${params}:${escaped}`, ...lines.slice(at + 1)].join('\r\n');
}
// Appended inside the component, immediately after UID, so it lands in the right block of a file that
// holds more than one.
const uidAt = lines.findIndex((l) => l.startsWith('UID:'));
const insertAt = uidAt === -1 ? Math.max(0, lines.length - 2) : uidAt + 1;
return [...lines.slice(0, insertAt), `${prop}:${escaped}`, ...lines.slice(insertAt)].join('\r\n');
}
export function useDavEdit(invalidateKey: unknown[]) {
const client = useClient();
const qc = useQueryClient();
const [busy, setBusy] = useState<string | null>(null);
const [error, setError] = useState<string | null>(null);
const refresh = () => qc.invalidateQueries({ queryKey: invalidateKey });
/** The object's original text, so a caller can patch rather than rebuild. */
const readRaw = async (href: string): Promise<string> => {
const res = await client.get<{ data: string }>(`/caldav/_officer/item?href=${encodeURIComponent(href)}`);
return res.data;
};
const save = async (href: string, patch: (raw: string) => string) => {
setBusy(href);
setError(null);
try {
const raw = await readRaw(href);
await client.put('/caldav/_officer/item', { href, data: patch(raw) });
await refresh();
} catch (err) {
setError(err instanceof Error ? err.message : 'Could not save.');
} finally {
setBusy(null);
}
};
const remove = async (href: string) => {
setBusy(href);
setError(null);
try {
await client.delete('/caldav/_officer/item', { href });
await refresh();
} catch (err) {
setError(err instanceof Error ? err.message : 'Could not delete.');
} finally {
setBusy(null);
}
};
return { save, remove, readRaw, busy, error };
}