fix(web,server): a direct item share labels and colours its field chips like a collection share, from defs that disclose only what the item shows (TASK-2248 U3) (#1587)

* fix(web,server): a direct item share labels and colours its field chips like a collection share, from defs that disclose only what the item shows (TASK-2248 U3)

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9

* test(server): the U3 leak scan says what it asserts, and names the label/type disclosure as the reviewed decision (TASK-2248 U3, codex round 1)

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
This commit is contained in:
xarmian
2026-09-26 20:13:56 -04:00
committed by GitHub
parent 7d89e352c8
commit c9c299ff18
8 changed files with 395 additions and 106 deletions
+71
View File
@@ -390,6 +390,68 @@ func (s *Server) requireShareLinkTargetVisible(w http.ResponseWriter, r *http.Re
// `writeJSON(w, ..., item)` would publish destinations on every public share
// link. TestMovedTo_ShareLinkNeverCarriesPointer pins both the key set and
// that specific omission.
// publicShareFieldDef is what a single-item share tells its reader about one
// field (TASK-2248 U3, audit C121). The direct item share used to carry no
// schema at all, so its field chips printed raw keys (`in_progress`) with no
// colour, while the same item expanded in a COLLECTION share rendered them
// properly. It is deliberately NOT the FieldDef: a link to one item never
// disclosed its collection's schema, so this carries only what renders the
// values the item already shows. There are no option lists, no relation
// target, no defaults and no validation rules. TerminalOptions is at most the
// item's own current value, and only when that value is terminal, because
// that is the one fact the chip colour needs.
type publicShareFieldDef struct {
Key string `json:"key"`
Label string `json:"label"`
Type string `json:"type"`
TerminalOptions []string `json:"terminal_options,omitempty"`
}
// publicShareFieldDefs returns, in schema order, the defs for the fields the
// item CARRIES A VALUE for. Computed fields are left out, as the collection
// share's expansion leaves them out. Returns nil when there is nothing to
// describe, or when the schema or the fields do not parse, so the caller omits
// the key and the page falls back to printing the values as it always did.
func publicShareFieldDefs(schemaJSON, fieldsJSON string) []publicShareFieldDef {
s := strings.TrimSpace(schemaJSON)
f := strings.TrimSpace(fieldsJSON)
if s == "" || f == "" {
return nil
}
var schema models.CollectionSchema
if err := models.UnmarshalItemFieldSchema([]byte(s), &schema); err != nil {
return nil
}
var values map[string]any
if err := json.Unmarshal([]byte(f), &values); err != nil {
return nil
}
var defs []publicShareFieldDef
for _, fd := range schema.Fields {
if fd.Computed {
continue
}
v, ok := values[fd.Key]
if !ok || v == nil {
continue
}
if str, isStr := v.(string); isStr && strings.TrimSpace(str) == "" {
continue
}
def := publicShareFieldDef{Key: fd.Key, Label: fd.Label, Type: fd.Type}
if str, isStr := v.(string); isStr {
for _, t := range fd.TerminalOptions {
if t == str {
def.TerminalOptions = []string{str}
break
}
}
}
defs = append(defs, def)
}
return defs
}
func publicShareItemDTO(item *models.Item) map[string]interface{} {
dto := map[string]interface{}{
"title": item.Title,
@@ -555,6 +617,15 @@ func (s *Server) handleResolveShareLink(w http.ResponseWriter, r *http.Request)
if refs := s.mintShareAttachmentRefs(link, item.Content); refs != nil {
resp["attachment_refs"] = refs
}
// Beside the item, not inside it: the item DTO's key set is pinned
// (TestMovedTo_ShareLinkNeverCarriesPointer). Scoped by the item's
// own collection ID. A lookup failure omits the key rather than
// failing the share, which then renders exactly as it did before U3.
if coll, err := s.store.GetCollection(item.CollectionID); err == nil && coll != nil {
if defs := publicShareFieldDefs(coll.Schema, item.Fields); defs != nil {
resp["field_defs"] = defs
}
}
writeJSON(w, http.StatusOK, resp)
case "collection":
@@ -0,0 +1,106 @@
package server
import (
"encoding/json"
"net/http"
"reflect"
"strings"
"testing"
"github.com/PerpetualSoftware/pad/internal/models"
)
// TASK-2248 U3 (audit C121). A single-item share carries field defs for the
// fields the item shows, so its chips can be labelled and coloured, and
// carries nothing else about the schema. A link to one item never disclosed its
// collection's option lists, relation targets, defaults or rules, and must not
// start to.
const u3Schema = `{"fields":[
{"key":"status","label":"Status","type":"select","options":["todo","doing","shipped","dropped"],"terminal_options":["shipped","dropped"],"abandoned_options":["dropped"],"default":"todo","required":true},
{"key":"owner","label":"Owner","type":"relation","collection":"people"},
{"key":"secret_codename","label":"Codename","type":"text"},
{"key":"score","label":"Score","type":"number","computed":true},
{"key":"effort","label":"Effort","type":"select","options":["s","m","l"]}
]}`
func shareFieldDefsFor(t *testing.T, srv *Server, wsID, collID, ownerID, fields string) (map[string]any, []map[string]any) {
t.Helper()
it, err := srv.store.CreateItem(wsID, collID, models.ItemCreate{Title: "Shared", Fields: fields})
if err != nil {
t.Fatalf("create item: %v", err)
}
link, err := srv.store.CreateShareLink(wsID, "item", it.ID, "view", ownerID, nil)
if err != nil {
t.Fatalf("create share link: %v", err)
}
rr := doRequest(srv, "GET", "/api/v1/s/"+link.Token, nil)
if rr.Code != http.StatusOK {
t.Fatalf("resolve share: %d: %s", rr.Code, rr.Body.String())
}
var raw map[string]any
if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil {
t.Fatalf("decode: %v", err)
}
var defs []map[string]any
if d, ok := raw["field_defs"]; ok {
b, _ := json.Marshal(d)
_ = json.Unmarshal(b, &defs)
}
return raw, defs
}
func TestItemShareCarriesDefsOnlyForTheFieldsItShows(t *testing.T) {
srv := testServer(t)
slug := createWSWithCollections(t, srv)
ws, err := srv.store.GetWorkspaceBySlug(slug)
if err != nil || ws == nil {
t.Fatalf("get workspace: %v", err)
}
owner, err := srv.store.CreateUser(models.UserCreate{Email: "u3-owner@test.com", Name: "Owner", Password: "pw-owner"})
if err != nil {
t.Fatalf("create user: %v", err)
}
coll, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{Name: "Features", Schema: u3Schema})
if err != nil {
t.Fatalf("create collection: %v", err)
}
t.Run("a terminal value: its own def, in schema order, and nothing else", func(t *testing.T) {
raw, defs := shareFieldDefsFor(t, srv, ws.ID, coll.ID, owner.ID, `{"status":"shipped","effort":"m","score":7,"secret_codename":""}`)
want := []map[string]any{
{"key": "status", "label": "Status", "type": "select", "terminal_options": []any{"shipped"}},
{"key": "effort", "label": "Effort", "type": "select"},
}
if !reflect.DeepEqual(defs, want) {
t.Fatalf("field_defs = %#v\nwant %#v", defs, want)
}
// The label and type of each shown field ARE schema facts, sent on
// purpose: that disclosure is the reviewed decision in #1587, and the
// DeepEqual above pins exactly what it is. What this scan adds is
// narrower. None of the listed schema facts beyond those appears
// anywhere in the response: option lists, the other options,
// abandoned options, defaults, required, the relation target, and the
// labels of fields this item carries no value for.
body, _ := json.Marshal(raw)
for _, leak := range []string{`"options"`, "doing", "dropped", "abandoned_options", `"default"`, `"required"`, "people", "Codename", "Score"} {
if strings.Contains(string(body), leak) {
t.Errorf("the item share discloses %s: %s", leak, body)
}
}
})
t.Run("a non-terminal value carries no terminal list at all", func(t *testing.T) {
_, defs := shareFieldDefsFor(t, srv, ws.ID, coll.ID, owner.ID, `{"status":"doing"}`)
want := []map[string]any{{"key": "status", "label": "Status", "type": "select"}}
if !reflect.DeepEqual(defs, want) {
t.Fatalf("field_defs = %#v\nwant %#v", defs, want)
}
})
t.Run("an item with no values carries no key, so the page renders as before", func(t *testing.T) {
raw, _ := shareFieldDefsFor(t, srv, ws.ID, coll.ID, owner.ID, `{}`)
if _, ok := raw["field_defs"]; ok {
t.Fatalf("field_defs present on an item with no values: %v", raw["field_defs"])
}
})
}
@@ -0,0 +1,61 @@
<script lang="ts">
// The field chips of a shared item, one component for both share views
// (TASK-2248 U3): the collection inline-expand and the direct item share.
// Presentational only. `fieldChips` in shareView.ts decides what shows and
// how it reads.
import { formatLabel, PUBLIC_RELATION_TITLE, type FieldChip } from './shareView';
let { chips }: { chips: FieldChip[] } = $props();
</script>
{#if chips.length > 0}
<dl class="public-field-chips">
{#each chips as { field, text, placeholder, color } (field.key)}
<div class="field-chip">
<dt class="field-chip-label">{field.label || formatLabel(field.key)}</dt>
{#if placeholder}
<dd class="field-chip-value is-placeholder" title={PUBLIC_RELATION_TITLE}>{text}</dd>
{:else}
<dd class="field-chip-value" style:color>{text}</dd>
{/if}
</div>
{/each}
</dl>
{/if}
<style>
.public-field-chips {
display: flex;
flex-wrap: wrap;
gap: var(--space-2);
margin: 0;
}
.field-chip {
display: inline-flex;
align-items: center;
gap: var(--space-1);
padding: var(--space-1) var(--space-3);
background: var(--bg-tertiary);
border-radius: 999px;
font-size: 0.82em;
}
.field-chip-label {
color: var(--text-muted);
font-weight: 500;
margin: 0;
}
.field-chip-value {
color: var(--text-primary);
margin: 0;
}
/* A value this share cannot resolve (BUG-3016): reads as a note about the
field rather than as its content. Matches PublicTableView's cell. */
.field-chip-value.is-placeholder {
color: var(--text-muted);
font-style: italic;
}
</style>
@@ -12,15 +12,8 @@
// place avoids introducing a second, divergent {@html} source.
import type { FieldDef } from '$lib/types';
import type { PublicItem } from './shareView';
import {
visibleFields,
formatLabel,
formatFieldValue,
fieldValueColor,
publicRelationText,
PUBLIC_RELATION_TITLE,
} from './shareView';
import { isRelationType } from '$lib/items/relationFieldTypes';
import { fieldChips } from './shareView';
import PublicFieldChips from './PublicFieldChips.svelte';
import StaleBodyNotice from '$lib/components/common/StaleBodyNotice.svelte';
import { mermaidBlocks } from './shareMermaid';
@@ -36,67 +29,14 @@
let { item, fields, html, id }: Props = $props();
// Schema fields that carry a value on this item, in schema order. Computed
// fields are dropped (visibleFields) — they aren't part of the shared
// snapshot's meaningful data.
// A relation NEVER prints its stored value here (BUG-3016). The value is an
// item id and a share payload has no index behind it, so the entry says what
// it holds — the same placeholder the shared table renders, from the same
// helper. `publicRelationText` carries why resolving it is a visibility
// question rather than a payload one; IDEA-3066 is the unit that would.
let displayFields = $derived(
visibleFields(fields)
.map((f) => ({
field: f,
value: isRelationType(f.type)
? publicRelationText(f, item.fields[f.key])
: formatFieldValue(item.fields[f.key]),
placeholder: isRelationType(f.type)
}))
.filter((entry) => entry.value !== '')
);
// Categorical fields (status/priority/select) carry kebab/snake option keys
// the owner sees as title-cased labels — so we label-format + color those.
// Everything else is a LITERAL value (dates, IDs, slugs, URLs, free text)
// and must render verbatim — title-casing `2026-05-31` or `api_token` would
// corrupt it. Mirrors the single-item share view, which renders field values
// raw.
function isCategorical(field: FieldDef): boolean {
return field.key === 'status' || field.key === 'priority' || field.type === 'select';
}
function colorFor(field: FieldDef): string | undefined {
const raw = item.fields[field.key];
if (typeof raw !== 'string') return undefined;
if (isCategorical(field)) return fieldValueColor(field, raw);
return undefined;
}
function displayValue(field: FieldDef, value: string): string {
// Title Case for categorical values — matches the chip pills on the
// card/list/table surfaces (PLAN-2290 Phase 3; no more SHOUTED status).
const base = isCategorical(field) ? formatLabel(value) : value;
return field.suffix ? `${base} ${field.suffix}` : base;
}
// Schema fields that carry a value on this item, in schema order, as the
// shared rule builds them. The direct item share renders through the same
// rule (TASK-2248 U3); `fieldChips` carries the reasoning.
let chips = $derived(fieldChips(item.fields, fields));
</script>
<div class="item-expansion" {id} role="region" aria-label="{item.title} details">
{#if displayFields.length > 0}
<dl class="expansion-fields">
{#each displayFields as { field, value, placeholder } (field.key)}
{@const color = colorFor(field)}
<div class="field-chip">
<dt class="field-chip-label">{field.label || formatLabel(field.key)}</dt>
{#if placeholder}
<dd class="field-chip-value is-placeholder" title={PUBLIC_RELATION_TITLE}>{value}</dd>
{:else}
<dd class="field-chip-value" style:color>{displayValue(field, value)}</dd>
{/if}
</div>
{/each}
</dl>
{/if}
<PublicFieldChips {chips} />
{#if html}
{#if item.contentStale}<StaleBodyNotice />{/if}
@@ -105,7 +45,7 @@
<div class="expansion-content" {@attach mermaidBlocks(html)}>
{@html html}
</div>
{:else if displayFields.length === 0}
{:else if chips.length === 0}
<p class="expansion-empty">No additional details.</p>
{/if}
</div>
@@ -124,41 +64,6 @@
border-radius: 0 0 var(--radius-lg) var(--radius-lg);
}
.expansion-fields {
display: flex;
flex-wrap: wrap;
gap: var(--space-2);
margin: 0;
}
.field-chip {
display: inline-flex;
align-items: center;
gap: var(--space-1);
padding: var(--space-1) var(--space-3);
background: var(--bg-tertiary);
border-radius: 999px;
font-size: 0.82em;
}
.field-chip-label {
color: var(--text-muted);
font-weight: 500;
margin: 0;
}
.field-chip-value {
color: var(--text-primary);
margin: 0;
}
/* A value this share cannot resolve (BUG-3016) — reads as a note about the
field rather than as its content. Matches PublicTableView's cell. */
.field-chip-value.is-placeholder {
color: var(--text-muted);
font-style: italic;
}
.expansion-empty {
color: var(--text-muted);
font-size: 0.88em;
+50
View File
@@ -482,3 +482,53 @@ export function groupItems(
}
return result;
}
/** One field chip as a share renders it (see `fieldChips`). */
export interface FieldChip {
field: FieldDef;
/** What the chip prints: label-cased for a categorical value, with any suffix. */
text: string;
/** A relation, which a share cannot resolve; printed as a note (BUG-3016). */
placeholder: boolean;
color?: string;
}
// Categorical fields (status/priority/select) carry kebab/snake option keys
// the owner sees as title-cased labels, so those are label-formatted and
// coloured. Everything else is a LITERAL value (dates, IDs, slugs, URLs, free
// text) and renders verbatim: title-casing `2026-05-31` or `api_token` would
// corrupt it.
function isCategorical(field: FieldDef): boolean {
return field.key === 'status' || field.key === 'priority' || field.type === 'select';
}
/**
* The chips a shared item shows: schema fields carrying a value, in schema
* order, computed fields dropped. ONE rule for both share views (TASK-2248
* U3). The collection inline-expand and the direct item share used to
* disagree, and the direct share printed raw keys (`in_progress`) with no
* colour. A relation never prints its stored value, which is an item id a
* share has no index for (BUG-3016).
*/
export function fieldChips(values: Record<string, unknown>, fields: FieldDef[]): FieldChip[] {
const out: FieldChip[] = [];
for (const field of visibleFields(fields)) {
const raw = values[field.key];
if (isRelationType(field.type)) {
const text = publicRelationText(field, raw);
if (text !== '') out.push({ field, text, placeholder: true });
continue;
}
const value = formatFieldValue(raw);
if (value === '') continue;
const categorical = isCategorical(field);
const base = categorical ? formatLabel(value) : value;
out.push({
field,
text: field.suffix ? `${base} ${field.suffix}` : base,
placeholder: false,
color: categorical && typeof raw === 'string' ? fieldValueColor(field, raw) : undefined,
});
}
return out;
}
+4
View File
@@ -138,6 +138,10 @@ export interface SharePayload {
/** BUG-3000 marker: `content` is behind the live document (BUG-3050 U3). */
content_state?: 'applied_pending_flush';
};
/** Single-item shares only (TASK-2248 U3): defs for the fields the item
* shows, in schema order. Deliberately NOT the schema: key, label, type,
* and terminal_options holding at most the item's own current value. */
field_defs?: Pick<FieldDef, 'key' | 'label' | 'type' | 'terminal_options'>[];
collection?: PublicShareCollection;
items?: PublicShareItem[];
// Renderable image-attachment refs embedded in the shared content
+17 -3
View File
@@ -16,10 +16,13 @@
parsePublicItems,
filterEvaluable,
matchesFilter,
fieldChips,
type PublicCollection,
type PublicItem
} from '$lib/components/share/shareView';
import PublicFieldChips from '$lib/components/share/PublicFieldChips.svelte';
import type {
FieldDef,
PublicShareCollection,
PublicShareItem,
PublicShareView,
@@ -66,6 +69,9 @@
collection_name?: string;
collection_icon?: string;
item_ref?: string;
/** Defs for the fields this item shows (TASK-2248 U3). Empty from an
* older server, which then renders the values raw, as before. */
fieldDefs: FieldDef[];
} | null>(null);
// Raw `collection` + `items` branches of the share payload, fed straight to
// PublicCollectionView (which parses settings/schema/fields defensively and
@@ -444,7 +450,8 @@
contentStale: isBodyStale(data.item),
collection_name: data.item?.collection_name,
collection_icon: data.item?.collection_icon,
item_ref: data.item?.ref ?? data.item?.item_ref
item_ref: data.item?.ref ?? data.item?.item_ref,
fieldDefs: Array.isArray(data.field_defs) ? data.field_defs : []
};
} else if (data.type === 'collection') {
shareType = 'collection';
@@ -520,7 +527,8 @@
contentStale: isBodyStale(data.item),
collection_name: data.item?.collection_name,
collection_icon: data.item?.collection_icon,
item_ref: data.item?.ref ?? data.item?.item_ref
item_ref: data.item?.ref ?? data.item?.item_ref,
fieldDefs: Array.isArray(data.field_defs) ? data.field_defs : []
};
} else if (data.type === 'collection') {
shareType = 'collection';
@@ -614,7 +622,13 @@
<h1 class="item-title">{itemData.title}</h1>
{#if Object.keys(itemData.fields).length > 0}
{#if itemData.fieldDefs.length > 0}
<!-- The collection inline-expand's rule and component (TASK-2248 U3):
labelled, coloured, relations as a note. -->
<div class="item-fields">
<PublicFieldChips chips={fieldChips(itemData.fields, itemData.fieldDefs)} />
</div>
{:else if Object.keys(itemData.fields).length > 0}
<div class="item-fields">
{#each Object.entries(itemData.fields) as [key, value] (key)}
{#if value !== null && value !== undefined && value !== ''}
@@ -0,0 +1,78 @@
// TASK-2248 U3 (audit C121): a direct item share renders its field chips by the
// same rule as the collection inline-expand (labelled, coloured, relations as
// a note) whenever the server sends defs, and exactly as before when it does
// not (an older server).
import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from '@testing-library/svelte';
import { fieldChips } from '$lib/components/share/shareView';
import type { FieldDef } from '$lib/types';
const share = vi.hoisted(() => ({ payload: null as unknown }));
vi.mock('$lib/api/client', () => ({
api: { share: { get: vi.fn(async () => share.payload) } },
withRequestDeadline: vi.fn(),
}));
import { page } from '$app/state';
import SharePage from './+page.svelte';
afterEach(() => cleanup());
async function settle() {
for (let i = 0; i < 20; i++) await Promise.resolve();
}
const defs = [
{ key: 'status', label: 'Status', type: 'select', terminal_options: ['shipped'] },
{ key: 'owner', label: 'Owner', type: 'relation' },
{ key: 'effort', label: 'Effort', type: 'select' },
{ key: 'hours', label: 'Hours', type: 'number', suffix: 'h' },
] as FieldDef[];
describe('fieldChips: one rule for both share views', () => {
it('labels a categorical value, colours a terminal one, notes a relation, keeps a literal verbatim', () => {
const chips = fieldChips({ status: 'shipped', owner: 'u-123', effort: 'in_progress', hours: 3, extra: 'x' }, defs);
expect(chips.map((c) => [c.field.key, c.text, c.placeholder])).toEqual([
['status', 'Shipped', false],
['owner', expect.not.stringContaining('u-123'), true],
['effort', 'In Progress', false],
['hours', '3 h', false],
]);
expect(chips[0].color).toBe('var(--accent-green)');
// An undeclared key has no def, so it is not a chip.
expect(chips.some((c) => c.field.key === 'extra')).toBe(false);
});
it('a computed field is not a chip', () => {
expect(fieldChips({ score: 7 }, [{ key: 'score', label: 'Score', type: 'number', computed: true } as FieldDef])).toEqual([]);
});
});
describe('share page: direct item field chips (C121)', () => {
function mount(extra: Record<string, unknown>) {
page.params = { token: 'tok' };
share.payload = {
type: 'item',
item: { title: 'Feature', ref: 'FEAT-1', fields: JSON.stringify({ status: 'shipped', effort: 'in_progress', owner: 'u-123' }), content: '' },
...extra,
};
return render(SharePage);
}
const chipTexts = (c: HTMLElement) => [...c.querySelectorAll('.item-fields .field-chip')].map((el) => el.textContent?.replace(/\s+/g, ' ').trim());
it('with defs from the server: labelled, coloured, the relation as a note and never its id', async () => {
const { container } = mount({ field_defs: defs });
await settle();
expect(chipTexts(container)[0]).toBe('Status Shipped');
expect(chipTexts(container)).toContain('Effort In Progress');
expect(container.querySelector('.item-fields')!.textContent).not.toContain('u-123');
const status = [...container.querySelectorAll('.field-chip-value')].find((d) => d.textContent === 'Shipped') as HTMLElement;
expect(status.style.color).toBe('var(--accent-green)');
});
it('without defs (an older server): the raw values, as before', async () => {
const { container } = mount({});
await settle();
expect(chipTexts(container)).toContain('Effort in_progress');
});
});