Fix #391: visibility save hardening #392

Merged

crueber merged 1 commit from fix/issue-391 into main

2026-09-12 16:37:52 +00:00

crueber commented

2026-09-12 16:35:17 +00:00

Owner

Root cause (evidence, not inference). One-line access GET/PUT outcome logs added (internal/identity/access.go GetAccess source/version/visibility at Debug; routeAccess GET denied/error at Info, PUT denied/conflict/error/success with version-in → version-out at Info — e.g. identity: access PUT conflict repo=acme/repo have=0 current=1). New handler-level evidence tests (internal/identity/access_visibility_391_test.go) run the real HTTP path against real memory + filesystem backends and show:

fresh-version PUT sticks across refreshes incl. the LRU NotModified re-read;
a stale integer version deterministically 409s; a non-admin PUT deterministically 403s;
two instances sharing one store converge on the next conditional revalidation.
So candidates #3 (store conditional staleness) and #4 (multi-instance disagreement) do NOT reproduce on these backend classes — the store contract suite pins the If-None-Match mapping they depend on. The defect is candidates #1+#2: the old Settings saveVisibility caught 403/409 into a small note and LEFT the user's chosen value in the select; the next refresh reseeded server truth = the bounce.

Fix.

web/src/lib/accessSave.js (new, shared): saveVisibilityOnly — fresh access.get() version immediately before each PUT, one re-read retry on 409; reseedVisibility — server truth for the failure path; friendlyAccessError — specific wording shared by both tabs.
Settings.jsx saveVisibility: on ANY failure reseed the select from server truth + specific note (never shows an unsaved value); Access.jsx keeps its no-retry full-document PUT (a blind retry there would clobber a concurrent binding edit) and reuses the shared wording.
03_store_backends.md: conditional-GET contract (§2.2 addition + Decisions entry) the identity LRU relies on — 304 iff version matches, 200-with-body otherwise, no intermediary may synthesize 304.
features/01_identity_permissions.md: Decisions entry.

Tests. Go: 3 new tests x2 backends, -race, count=5 clean; identity coverage 95.4% (gate holds); store contract suite green; gofmt/vet/build clean. Node: 7 new access-save tests + full unit suite 764/766 (2 failures are the pre-existing smoke dist-build tests, identical on clean main). No new deps. S3 rig skipped (not quick); S3 unsigned-header path unchanged, now contract-documented.

Follow-up to #381: the visibility select still bounced after save. The #381 HTTP-cache explanation no longer covers it (#382/#384/#385 moved every surface off stale-serve), so this instruments first and fixes the failure path with evidence. **Root cause (evidence, not inference).** One-line access GET/PUT outcome logs added (internal/identity/access.go GetAccess source/version/visibility at Debug; routeAccess GET denied/error at Info, PUT denied/conflict/error/success with version-in → version-out at Info — e.g. `identity: access PUT conflict repo=acme/repo have=0 current=1`). New handler-level evidence tests (internal/identity/access_visibility_391_test.go) run the real HTTP path against real memory + filesystem backends and show: - fresh-version PUT sticks across refreshes incl. the LRU NotModified re-read; - a stale integer version deterministically 409s; a non-admin PUT deterministically 403s; - two instances sharing one store converge on the next conditional revalidation. So candidates #3 (store conditional staleness) and #4 (multi-instance disagreement) do NOT reproduce on these backend classes — the store contract suite pins the If-None-Match mapping they depend on. The defect is candidates #1+#2: the old Settings saveVisibility caught 403/409 into a small note and LEFT the user's chosen value in the select; the next refresh reseeded server truth = the bounce. **Fix.** - web/src/lib/accessSave.js (new, shared): saveVisibilityOnly — fresh access.get() version immediately before each PUT, one re-read retry on 409; reseedVisibility — server truth for the failure path; friendlyAccessError — specific wording shared by both tabs. - Settings.jsx saveVisibility: on ANY failure reseed the select from server truth + specific note (never shows an unsaved value); Access.jsx keeps its no-retry full-document PUT (a blind retry there would clobber a concurrent binding edit) and reuses the shared wording. - 03_store_backends.md: conditional-GET contract (§2.2 addition + Decisions entry) the identity LRU relies on — 304 iff version matches, 200-with-body otherwise, no intermediary may synthesize 304. - features/01_identity_permissions.md: Decisions entry. **Tests.** Go: 3 new tests x2 backends, -race, count=5 clean; identity coverage 95.4% (gate holds); store contract suite green; gofmt/vet/build clean. Node: 7 new access-save tests + full unit suite 764/766 (2 failures are the pre-existing smoke dist-build tests, identical on clean main). No new deps. S3 rig skipped (not quick); S3 unsigned-header path unchanged, now contract-documented.

crueber added 1 commit

2026-09-12 16:35:17 +00:00

Fix #391 : visibility save hardening (features/01 Decisions; 03 §2.2 conditional contract; identity access logging; web accessSave shared path) e133fe2ecd

crueber referenced this pull request

2026-09-12 16:35:22 +00:00

Visibility still bounces after #381: settings save may silently fail (403/409 with no reseed), stale CAS version, or store conditional-GET staleness #391

crueber referenced this pull request

2026-09-12 16:37:33 +00:00

Visibility still bounces after #381: settings save may silently fail (403/409 with no reseed), stale CAS version, or store conditional-GET staleness #391

crueber merged commit d174bf68a8 into main

2026-09-12 16:37:52 +00:00

crueber referenced this pull request from a commit