Repository navigation
download-authz-both-sides c4: a 403 attachment list is rendered as "No attachments yet" #4269
Description
Symptom
A member who is denied the parent record opens a record that holds 2095+ attachments. The attachments panel fires GET /api/v1/data/sys_attachment?...filter=parent_object=…, the server returns 403, and the panel renders its empty state — "No attachments yet. Upload a file to get started." — and still advertises an Upload affordance the server would refuse. An operator cannot tell "nothing here" from "not yours".
Root cause
Not located by the run (objectui internals). The panel swallows a 40x list response into its empty state instead of mapping it to a distinct "denied" state. Nothing leaks — the objectui#2532 failure mode (raw error dump / leaked row) is absent, and no status code is shown; the gap is purely surfacing: a 40x is folded into the empty state rather than a denied state.
Reproduction
As a member denied READ on the parent, open a record that in fact holds attachments. The list request returns 403 and the panel shows the empty state.
- Expected: denial copy (a "you don't have access to these attachments" state), and no Upload affordance.
- Actual: "No attachments yet. Upload a file to get started." + an Upload affordance, on a record that holds 2095+ attachments.
Source
Extracted from the QA run objectstack-ai/objectstack#7635 (framework 92f26f75, console 6314e87).
Activity
yinlianghui commentedon Aug 15, 2026
Claim: PM loop round 8
Session:
session_01RnQd8iMMUwXQEV1crFmQiQBranch:
claude/issue-4269-attachments-denied-stateWorktree:
objectui-issue-4269Domain:
repo:objectuiFile surface: the attachments panel component that issues the
sys_attachmentlist read (exact file per re-derivation — expected in plugin-detail or components) + its tests + the new denied-state i18n keys (en + all ten locale packs, per the all-locales parity gate) (stop on breach; explain in the report)Container & model: S–M,
mode:subagent,model: opusSerial constraints cleared: #4672 holds plugin-charts, #4673 holds core chart-series, #4296 holds plugin-grid — disjoint; locale packs are free (#4675 merged); no open dev PR touches the attachments panel (checked this round).
Generated by Claude Code
yinlianghui commentedon Aug 15, 2026
{ "issue": 4269, "status": "done", "branch": "claude/issue-4269-attachments-denied-state", "pr": "https://github.com/objectstack-ai/objectui/pull/4685", "premise_still_valid": true, "summary": "Re-derived all three premises on current main (b8818171f) before building. Premise A holds and is now measured precisely: a 403 on the sys_attachment list read THROWS — ObjectStackAdapter.find() degrades only a non-authz 404 to {data: [], total: 0} and rethrows the rest (objectui#4408's carve-out) — so RecordAttachmentsPanel's bare `catch { setRows([]) }` was where 'denied' and 'empty' merged. Premise B: the Upload affordance was keyed on NOTHING (only `disabled={uploading}`), so withdrawing it under a denied list disturbs no existing permission-based visibility. Premise C: a house pattern exists and was mirrored rather than reinvented — `isPermissionError` from @object-ui/react (403 / PERMISSION_DENIED / FORBIDDEN / RLS), the same predicate ObjectKanban, ObjectCalendar and the form renderer branch on, plus ObjectDataPage's Lock-icon denial framing. The panel now renders a distinct denied state ('You don't have access to these attachments.', new detail.attachmentsAccessDenied key in en + all ten locale packs) with no Upload affordance, checked before the empty state; the empty state is reserved for a genuine 200-with-zero-rows. #2532's leak mode stays absent — the denied state renders the i18n sentence and nothing from the error, and setError is deliberately not called on that path. Non-authz failures keep today's handling, pinned by a test labelled as a pin, with the remaining swallow filed as #4684.", "tests": "All gates run AFTER the final commit, at HEAD 6e3013925 (working tree clean). (1) Panel tests — `pnpm exec vitest run packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx` => 'Test Files 1 passed (1) / Tests 14 passed (14)' (8 pre-existing + 6 new). (2) Full i18n suite (all ten packs touched) — `pnpm exec vitest run packages/i18n/` => '45 passed (45) / 804 passed (804)', includes all-locales-key-parity + en-zh-key-parity. (3) Build closure first: `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build` => exit 0. (4) `pnpm --filter @object-ui/app-shell --filter @object-ui/i18n type-check` => both 'Done' (NOTE: the script is `type-check`, not `typecheck`; the latter returned ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT — caught, not silently passed). (5) `eslint --quiet` over all 12 changed source files => exit 0. (6) `pnpm run check:i18n-keys` => every in-scope call-site key resolves against the en pack, every literal inline defaultValue matches its en value. (7) `pnpm run check:i18n-drift` => '1 key(s) added, 0 en value(s) changed'. (8) `pnpm run check:control-bytes` => OK, 4209 tracked text files. (9) `node scripts/check-changeset-presence.mjs` => '12 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)'. REVERSE VERIFICATION, direction predicted before running: reverting ONLY the component (tests kept) should turn exactly the 4 tests requiring the denied state red, while the 200-empty test and the non-authz PIN stay green (a pin asserts unchanged behaviour, so it must pass on both trees). Observed exactly that: 'Tests 4 failed | 10 passed (14)', and the failure dumps reproduce the reported symptom verbatim — on a 403 the reverted component rendered 'Upload' + 'No attachments yet. Upload a file to get started.' Restored byte-identically (`git diff HEAD` empty) and re-ran green at 14/14. Note: a repo guard (objectui#3288) rejected the first `pnpm --filter ... test -- --run path` form because vitest silently drops args after `--`; re-run in the prescribed repo-root form. No ablation in this change, so nothing is claimed about dist/.", "open_questions": [], "out_of_scope_findings": [ "filed as #4684: RecordAttachmentsPanel still renders 'No attachments yet' for a network/5xx/401 failure it never loaded — same defect class one status over, deliberately left by this card's ruling; needs the loaded/unknown status vocabulary HomeActionCenter (#4235), the app list (#4300) and ListView.classifyLoadError already carry. Unassigned, unlabeled for PM triage; the pin test in this PR is the one to rewrite when it is picked up." ] }Generated by Claude Code
yinlianghui commentedon Aug 15, 2026
ACCEPT — PR #4685 (reviewer of record; readings taken on GitHub at head
6e3013925, matching the report's gate-union HEAD).main, first lineFixes #4269— correct, full deliverable. No closing keyword near any other open card's number (RecordAttachmentsPanel renders "No attachments yet" for a network/5xx failure it never loaded #4684 referenced bare and explicitly "not fixed in this PR").OBJECT_API_DISABLEDrenders as the generic empty state — "this page cannot work" reads as "you have no records" #4408 carve-out re-verified at the adapter source), so the panel's barecatch { setRows([]) }is the measured merge point, not an assumed one. Premise B (Upload was gated on nothing) means the withdrawal disturbs no existing permission visibility. Premise C reuses the houseisPermissionErrorpredicate + Lock framing instead of inventing a second denial vocabulary.403, no row count, no user id, no server message, no alert role) — the denied state renders the i18n sentence and nothing sourced from the error.typecheckzero-match trap caught out loud rather than silently passed. Reverse verification predicted exactly 4 red / 10 green — observed exactly, with the failure dumps reproducing the reported symptom verbatim.CI on this head is converging — ready + auto-merge flips when Lint / Type Check / Test shards conclude green (covered by the standing timer round). Card closes on merge;
pm:dispatchedcomes off then. #4684 is graded on its own card.Generated by Claude Code