crueber/walhub

Watch

Fork 0

Visibility modes: private, logged-in only vs private, owner/org only (replaces the single members-only private) #374

New issue

Closed

opened 2026-09-12 13:35:13 +00:00 by crueber · 3 comments

crueber commented

2026-09-12 13:35:13 +00:00

Owner

What's requested

The repo Access tab's visibility control currently offers:

public
private — members only

"Members only" isn't the desired semantic. Replace the private option with two distinct private modes, chosen by who owns the repo:

Private, logged-in only — any authenticated user can read; anonymous visitors cannot. (Makes sense for repos owned by a user.)
Private, visible only by owner/org — only the owner themselves (for a user-owned repo) or the org's members (for an org-owned repo) can read. (Makes sense for repos owned by an organization.)

The UI should present the applicable options based on whether the repo's owner is a user or an organization (the org-vs-user distinction rides the #348 listing/profile work).

Current state (code evidence)

The Access tab (web/src/pages/Access.jsx:111) renders exactly two options: public and private — members only, posting visibility through the access API.
The server model has exactly two values: VisibilityPublic/VisibilityPrivate (internal/identity/identity.go:101-102, validVisibility :105-107, enforced in normalizeAccess access.go:62-64).
Resolution semantics for private today (Resolve, internal/identity/access.go:215-250): role comes from explicit RoleBindings (including team:<org>/<slug> subjects), org-owner implicit admin (:234-236), host-admin, or the host-wide p.Write fallback (:238-244). Anonymous gets read only when visibility is public (:246-248). So "private — members only" currently means: bindings + org owners + host admins + host-wide-write users — notably including any authenticated user with host-wide write, and excluding ordinary org members without bindings (they only get in via team: subjects or org-owner status).
The creation path accepts the same two values (internal/api/placeholder.go:381-382).

Proposed design

Three (or four) visibility values. Extend Visibility with:
- public (unchanged) — anonymous read allowed.
- authenticated (new) — "private, logged in only": any authenticated principal gets read; anonymous denied. Replaces the current intent of private for user-owned repos.
- private (semantics refined) — "visible only by owner/org": owner(s), org members, explicit bindings, host admin. For an org-owned repo this is the members-only mode the user wants; for a user-owned repo it means the owner + explicit bindings only.
- Wire-compat decision for the implementer: keep private as its own value (recommended — the enum is additive; old private docs gain the refined semantics) and add authenticated; the UI then shows the applicable pair per owner type. The old label "members only" disappears either way.
Resolution changes in Resolve:
- authenticated: after the existing binding/org-owner/admin resolution, an authenticated principal with no role gets RoleRead (insert before the anonymous-public branch; anonymous still denied).
- private (org-owned): org members (not just owners) get at least RoleRead — today an org member without a binding gets nothing, which contradicts "members only". Requires a membership check for the owner org (isOrgMember exists alongside isOrgOwner or via getMembers).
- private (user-owned): owner + bindings + admin only (current behavior, now correctly labeled).
- The host-wide p.Write fallback (:238-244) needs review against #347: a host-write-only outsider should probably NOT read private repos — that's part of the same rule tightening. Decision point for the user.
UI per owner type (Access tab): the visibility select offers public + the applicable private modes — for a user-owned repo: public / private — logged in only / private — owner only; for an org-owned repo: public / private — logged in only / private — org members only. The org-vs-user distinction comes from #348's owner-kind marker (or GetOrg existence as fallback).
Everywhere visibility values appear must accept the new enum: normalizeAccess/validVisibility (access.go:62-64), the create path (placeholder.go:381-382), the SDK typedef (web/sdk/src/types.js:85), and the settings copy. Listings/explore (#345 filtering) must treat authenticated like private for anonymous callers (hidden) and like public for authenticated ones (visible) — the listing filter needs the caller's auth state, which it has.
Migration: existing private repos keep private (now refined semantics) — but note the semantic change means org members gain access and host-write outsiders lose it; enumerate that behavioral delta in the PR and flag it for the user if the host-write fallback stays.

Acceptance criteria

Access tab offers the owner-appropriate visibility options (user-owned vs org-owned shapes above); "members only" wording is gone.
authenticated visibility: any signed-in user can read the repo (API + git + web), anonymous gets 401/404 per the #345 convention.
private on an org-owned repo: org members read without explicit bindings; non-members (even authenticated with host-wide write, per decision) denied.
private on a user-owned repo: owner + explicit bindings + admin only.
Listings (#345 filter) hide authenticated and private repos from anonymous callers; authenticated repos appear to any signed-in user.
New value accepted everywhere visibility is parsed (access normalize, create path, SDK typedef); old private docs migrate without data changes.
Resolution matrix tests per (visibility × owner-type × principal-class) cell; docs (01_identity_permissions.md) amended in the same change.

## What's requested The repo Access tab's visibility control currently offers: - `public` - `private — members only` "Members only" isn't the desired semantic. Replace the private option with **two distinct private modes**, chosen by who owns the repo: - **Private, logged-in only** — any authenticated user can read; anonymous visitors cannot. (Makes sense for repos owned by a **user**.) - **Private, visible only by owner/org** — only the owner themselves (for a user-owned repo) or the org's members (for an org-owned repo) can read. (Makes sense for repos owned by an **organization**.) The UI should present the applicable options based on whether the repo's owner is a user or an organization (the org-vs-user distinction rides the #348 listing/profile work). ## Current state (code evidence) - The Access tab (`web/src/pages/Access.jsx:111`) renders exactly two options: `public` and `private — members only`, posting `visibility` through the access API. - The server model has exactly two values: `VisibilityPublic`/`VisibilityPrivate` (`internal/identity/identity.go:101-102`, `validVisibility` :105-107, enforced in `normalizeAccess` `access.go:62-64`). - **Resolution semantics for `private` today** (`Resolve`, `internal/identity/access.go:215-250`): role comes from explicit `RoleBindings` (including `team:<org>/<slug>` subjects), org-owner implicit admin (`:234-236`), host-admin, or the host-wide `p.Write` fallback (:238-244). Anonymous gets read **only** when visibility is public (:246-248). So "private — members only" currently means: *bindings + org owners + host admins + host-wide-write users* — notably **including any authenticated user with host-wide `write`**, and *excluding* ordinary org members without bindings (they only get in via `team:` subjects or org-owner status). - The creation path accepts the same two values (`internal/api/placeholder.go:381-382`). ## Proposed design 1. **Three (or four) visibility values.** Extend `Visibility` with: - `public` (unchanged) — anonymous read allowed. - `authenticated` (new) — "private, logged in only": any authenticated principal gets read; anonymous denied. Replaces the current *intent* of `private` for user-owned repos. - `private` (semantics refined) — "visible only by owner/org": owner(s), org members, explicit bindings, host admin. For an org-owned repo this is the members-only mode the user wants; for a user-owned repo it means the owner + explicit bindings only. - Wire-compat decision for the implementer: keep `private` as its own value (recommended — the enum is additive; old `private` docs gain the refined semantics) and add `authenticated`; the UI then shows the applicable pair per owner type. The old label "members only" disappears either way. 2. **Resolution changes in `Resolve`:** - `authenticated`: after the existing binding/org-owner/admin resolution, an authenticated principal with no role gets `RoleRead` (insert before the anonymous-public branch; anonymous still denied). - `private` (org-owned): org **members** (not just owners) get at least `RoleRead` — today an org member without a binding gets nothing, which contradicts "members only". Requires a membership check for the owner org (`isOrgMember` exists alongside `isOrgOwner` or via `getMembers`). - `private` (user-owned): owner + bindings + admin only (current behavior, now correctly labeled). - The host-wide `p.Write` fallback (:238-244) needs review against #347: a host-write-only outsider should probably NOT read private repos — that's part of the same rule tightening. Decision point for the user. 3. **UI per owner type** (Access tab): the visibility select offers `public` + the applicable private modes — for a **user-owned** repo: `public` / `private — logged in only` / `private — owner only`; for an **org-owned** repo: `public` / `private — logged in only` / `private — org members only`. The org-vs-user distinction comes from #348's owner-kind marker (or `GetOrg` existence as fallback). 4. **Everywhere visibility values appear** must accept the new enum: `normalizeAccess`/`validVisibility` (`access.go:62-64`), the create path (`placeholder.go:381-382`), the SDK typedef (`web/sdk/src/types.js:85`), and the settings copy. Listings/explore (#345 filtering) must treat `authenticated` like `private` for anonymous callers (hidden) and like public for authenticated ones (visible) — the listing filter needs the caller's auth state, which it has. 5. **Migration**: existing `private` repos keep `private` (now refined semantics) — but note the semantic change means org members *gain* access and host-write outsiders *lose* it; enumerate that behavioral delta in the PR and flag it for the user if the host-write fallback stays. ## Acceptance criteria - [ ] Access tab offers the owner-appropriate visibility options (user-owned vs org-owned shapes above); "members only" wording is gone. - [ ] `authenticated` visibility: any signed-in user can read the repo (API + git + web), anonymous gets 401/404 per the #345 convention. - [ ] `private` on an org-owned repo: org members read without explicit bindings; non-members (even authenticated with host-wide write, per decision) denied. - [ ] `private` on a user-owned repo: owner + explicit bindings + admin only. - [ ] Listings (#345 filter) hide `authenticated` and `private` repos from anonymous callers; `authenticated` repos appear to any signed-in user. - [ ] New value accepted everywhere visibility is parsed (access normalize, create path, SDK typedef); old `private` docs migrate without data changes. - [ ] Resolution matrix tests per (visibility × owner-type × principal-class) cell; docs (01_identity_permissions.md) amended in the same change.

crueber added the

git storage

oidc

labels

2026-09-12 13:35:13 +00:00

crueber added this to the v1 milestone

2026-09-12 13:35:13 +00:00

crueber referenced this issue from a commit

2026-09-12 14:49:26 +00:00

Fix #374: visibility modes split (features/01 §4/§4.1/§9; 07_api §9)

crueber referenced this issue from a pull request that will close it,

2026-09-12 14:49:30 +00:00

Fix #374: visibility modes split #378

crueber commented

2026-09-12 14:49:35 +00:00

Author

Owner

Fix ready for review: #378 (branch fix/issue-374).

Rulings you asked for:

Host-write fallback: DENY (the #347 direction extended to reads). CheckRead/Resolve/listings no longer honor the write flag on private repos; delta documented (org members gain, host-write outsiders lose). One deliberate carve-out: repoimport checkCreate keeps its documented host-credential create-capability (roles consulted first; flag overcomes a per-repo Forbidden only).
Migration: no data change — existing private keeps its value under the refined semantics. Operators who used private as “all logged-in users” should flip those repos to authenticated.

All acceptance cells tested (matrix + Resolve roles + SSH e2e incl. a mallory-denied-over-SSH cell); gates green (33 pkgs ok, race on identity/api/server, coverage ≥95, fmt/vet, web 745/0 + vite/esbuild). Not merging — awaiting review.

Fix ready for review: #378 (branch fix/issue-374). Rulings you asked for: - Host-write fallback: DENY (the #347 direction extended to reads). CheckRead/Resolve/listings no longer honor the write flag on private repos; delta documented (org members gain, host-write outsiders lose). One deliberate carve-out: repoimport checkCreate keeps its documented host-credential create-capability (roles consulted first; flag overcomes a per-repo Forbidden only). - Migration: no data change — existing private keeps its value under the refined semantics. Operators who used private as “all logged-in users” should flip those repos to authenticated. All acceptance cells tested (matrix + Resolve roles + SSH e2e incl. a mallory-denied-over-SSH cell); gates green (33 pkgs ok, race on identity/api/server, coverage ≥95, fmt/vet, web 745/0 + vite/esbuild). Not merging — awaiting review.

crueber commented

2026-09-12 14:55:11 +00:00

Author

Owner

Review of PR #378 (fix/issue-374) — verified in scratch worktree at 9630c26. No browser (per instructions: tests + reasoning; web changes are select-option/badge logic covered by node unit tests).

ADVERSARIAL AUTH REVIEW — all 9 points hold:
(1) Enum additive: VisibilityAuthenticated added alongside public/private (identity.go); no stored value migrated, no field renumber (string enum). normalizeAccess/validVisibility accept 3 spellings; EnsureRepoAccess materializes authenticated; unknown still falls back to public on create path, 400 on POST/PUT. Old private docs untouched. PASS.
(2) Resolve order (access.go:304-341): bindings → orgRosterRole → authenticated → admin → anon-public. Bindings/roster/authenticated all inside if !p.Anonymous, so anonymous resolves nothing except public (gate.go:45-50: anon gets 401 unless public). Authenticated stranger on private resolves '' → 403. No anon-reads-non-public path; no authed-reads-owner-only-private path. PASS.
(3) Host-write retired everywhere on reads: CheckRead early-allows admin only (gate.go:41); Resolve grants admin only; unfiltered() admin-only (visibility.go) so writers pay per-repo CheckRead probes in listings. Grep p.Write on read paths: only /me echo (discovery.go:166) and the documented repoimport checkCreate carve-out (service.go:518: host flag overcomes per-repo Forbidden only, 401/503 surface, roles consulted first — correct for a create-capability). SSH fetch (bind_ssh.go:78), HTTP git (smart.go gateRepoRead), LFS (lfs.go:65), health probe all funnel through CheckRead. PASS, carve-out documented in code + 01 Decisions.
(4) Org-member private read: orgRosterRole (one members.json GET — same probe the old owner check paid, no added round trip) maps owner→admin, member→read; non-member/absent-org/error → false. Non-member with host-write denied (matrix cell org-priv-writer → 403). PASS.
(5) User-owned private: owner binding + explicit bindings + admin only; matrix pins owner-unbound denial even for the owner (fail closed). PASS.
(6) Listings: anon hides authenticated+private (CheckRead anon branch); signed-in sees authenticated (Resolve RoleRead). Writers now probed per repo — same probes ordinary users already pay, control-plane/SWR-cached, never on the law-6 budgeted paths (visibility.go header states this). No dedicated measurement, but structurally cost-neutral vs existing filtered callers. Acceptable.
(7) UI: Access.jsx + Settings.jsx probe owner kind via orgs.get 404→null (email owners skip via shouldFetchTeams); user gets owner-only label, org gets org-members-only label; New.jsx neutral triple (owner/org only). Old 'members only' label gone from selects/badge/SDK (residual hits are historical-rationale comments only). Loading-state flicker (undefined→user options→org options) is cosmetic; server accepts all spellings regardless of owner. PASS.
(8) No regress #345/#346/#347/#370: scriptedGate updated to admin-only bypass (host-write private → 403); push path untouched (CheckPush separate, host-write already stripped in #347); ETag ~v flows the visibility string; CheckCreateOwner untouched; matchPrincipal/username aliasing untouched. Pushguard SSH e2e owner-bind + mallory-denied cell passes. PASS.
(9) Migration delta enumerated + honest in 01 Decisions #374 entry: org members GAIN read, host-write outsiders LOSE private read, operator guidance to flip all-logged-in repos to authenticated. PASS.
(10) VERIFIED RESULTS: go build clean; gofmt clean; go vet clean (identity/api/server/repoimport); -race green identity, api, repoimport, server (server: only TestUIAssetConcepts fails — fails identically on clean main, missing concepts/.gif build artifact, pre-existing env); coverage identity 95.9%, api 95.2%, repoimport 95.7%, server 98.3% (all ≥95 gate); node visibility.test.js 4/4 pass, full web suite 746/748 (2 fails are smoke.test.js vs foreign :8080 instance — identical on main, PR-documented); TestPushGuard pass incl. new mallory cell.

NITS (no fix pushed): isOrgMemberFor is production-dead (only test caller; Resolve uses orgRosterRole directly) — harmless tested helper; writers-listing probe cost unmeasured (reasoned cost-neutral).

MERGE RECOMMENDATION: ready to merge.

Review of PR #378 (fix/issue-374) — verified in scratch worktree at 9630c26. No browser (per instructions: tests + reasoning; web changes are select-option/badge logic covered by node unit tests). ADVERSARIAL AUTH REVIEW — all 9 points hold: (1) Enum additive: VisibilityAuthenticated added alongside public/private (identity.go); no stored value migrated, no field renumber (string enum). normalizeAccess/validVisibility accept 3 spellings; EnsureRepoAccess materializes authenticated; unknown still falls back to public on create path, 400 on POST/PUT. Old private docs untouched. PASS. (2) Resolve order (access.go:304-341): bindings → orgRosterRole → authenticated → admin → anon-public. Bindings/roster/authenticated all inside if !p.Anonymous, so anonymous resolves nothing except public (gate.go:45-50: anon gets 401 unless public). Authenticated stranger on private resolves '' → 403. No anon-reads-non-public path; no authed-reads-owner-only-private path. PASS. (3) Host-write retired everywhere on reads: CheckRead early-allows admin only (gate.go:41); Resolve grants admin only; unfiltered() admin-only (visibility.go) so writers pay per-repo CheckRead probes in listings. Grep p.Write on read paths: only /me echo (discovery.go:166) and the documented repoimport checkCreate carve-out (service.go:518: host flag overcomes per-repo Forbidden only, 401/503 surface, roles consulted first — correct for a create-capability). SSH fetch (bind_ssh.go:78), HTTP git (smart.go gateRepoRead), LFS (lfs.go:65), health probe all funnel through CheckRead. PASS, carve-out documented in code + 01 Decisions. (4) Org-member private read: orgRosterRole (one members.json GET — same probe the old owner check paid, no added round trip) maps owner→admin, member→read; non-member/absent-org/error → false. Non-member with host-write denied (matrix cell org-priv-writer → 403). PASS. (5) User-owned private: owner binding + explicit bindings + admin only; matrix pins owner-unbound denial even for the owner (fail closed). PASS. (6) Listings: anon hides authenticated+private (CheckRead anon branch); signed-in sees authenticated (Resolve RoleRead). Writers now probed per repo — same probes ordinary users already pay, control-plane/SWR-cached, never on the law-6 budgeted paths (visibility.go header states this). No dedicated measurement, but structurally cost-neutral vs existing filtered callers. Acceptable. (7) UI: Access.jsx + Settings.jsx probe owner kind via orgs.get 404→null (email owners skip via shouldFetchTeams); user gets owner-only label, org gets org-members-only label; New.jsx neutral triple (owner/org only). Old 'members only' label gone from selects/badge/SDK (residual hits are historical-rationale comments only). Loading-state flicker (undefined→user options→org options) is cosmetic; server accepts all spellings regardless of owner. PASS. (8) No regress #345/#346/#347/#370: scriptedGate updated to admin-only bypass (host-write private → 403); push path untouched (CheckPush separate, host-write already stripped in #347); ETag ~v flows the visibility string; CheckCreateOwner untouched; matchPrincipal/username aliasing untouched. Pushguard SSH e2e owner-bind + mallory-denied cell passes. PASS. (9) Migration delta enumerated + honest in 01 Decisions #374 entry: org members GAIN read, host-write outsiders LOSE private read, operator guidance to flip all-logged-in repos to authenticated. PASS. (10) VERIFIED RESULTS: go build clean; gofmt clean; go vet clean (identity/api/server/repoimport); -race green identity, api, repoimport, server (server: only TestUIAssetConcepts fails — fails identically on clean main, missing concepts/*.gif build artifact, pre-existing env); coverage identity 95.9%, api 95.2%, repoimport 95.7%, server 98.3% (all ≥95 gate); node visibility.test.js 4/4 pass, full web suite 746/748 (2 fails are smoke.test.js vs foreign :8080 instance — identical on main, PR-documented); TestPushGuard* pass incl. new mallory cell. NITS (no fix pushed): isOrgMemberFor is production-dead (only test caller; Resolve uses orgRosterRole directly) — harmless tested helper; writers-listing probe cost unmeasured (reasoned cost-neutral). MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-12 14:55:41 +00:00

crueber commented

2026-09-12 14:55:41 +00:00

Author

Owner

Fixed by PR #378 (review clean — adversarial auth-semantics pass, host-write retired, no regress on #345/#346/#347/#370), merged. Closing.

crueber referenced this issue from a commit

2026-09-12 14:55:41 +00:00

Merge pull request 'Fix #374: visibility modes split' (#378) from fix/issue-374 into main

crueber referenced this issue

2026-09-13 23:23:14 +00:00