Portal: the per-share management switch, in the console and in the sharing table - #64
Closed
Daily-AC wants to merge 4 commits into
Closed
Portal: the per-share management switch, in the console and in the sharing table#64Daily-AC wants to merge 4 commits into
Daily-AC wants to merge 4 commits into
Conversation
…y, and two table defects (#59) * Hide the Invites tab from non-admin users The settings nav marks the Invites entry `hidden` and app.js only clears the attribute for `me.role === 'admin'`, but `.sgroup button{display:block}` in app.css has specificity (0,1,1) against the UA stylesheet's (0,1,0) `[hidden]{display:none}`. The attribute lost, so every user saw an entry that redirects to tokens and 403s at the relay. The same trap is documented a few hundred lines up in the comment on `.ava img`, which shows that remembering is not a mechanism. Add a global `[hidden]{display:none!important}` rather than a targeted `.sgroup button[hidden]` rule: measured across 72 headless renders (16 routes x 2 widths x 2 roles, plus the four auth pages), `#sInvites` is the only element in the portal whose `hidden` attribute is currently overridden, so the global rule fixes that one case and changes nothing else. Nothing in the portal shows an element by overriding `hidden` with `display`; `hidden` is only ever assigned to hide. Give the sweep a measurement for the whole class of bug rather than for this instance: element carries `hidden`, computed display is not `none`. Unlike the other six checks it has no acceptable non-zero value, so it names the offenders and sets a non-zero exit code. The fixtures gain `?role=user` and the state list a `settings-user` screen, without which no swept state has a hidden element to measure. Verified: with the CSS rule removed the sweep exits 1 and reports `#sInvites -> display:block`; with it, `#sInvites` computes to `display: none` for role=user and `block` for role=admin, on both sides of the 760px breakpoint. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e * Add the v0.7.0 changelog entry and refuse a release without one The portal's header badge is `CurrentVersion()`, the newest file in internal/portal/changelog/. PR #56 shipped v0.7.0 with no entry, so a correctly deployed v0.7.0 portal kept showing v0.6.1. Containers, manifest and .env were all correct; the badge was reading the only thing that had not been updated. Write the missing entry, covering what a user acts on: the agent updates itself every 6h, `auto_update` is on by default and turned off with `wanctl config set auto_update=off` or `WANCTL_AUTO_UPDATE`, an update never interrupts a live session, and a device that cannot reach GitHub falls back to the relay's /dl mirror. Android app, relay and portal are out of scope. Guard it in scripts/validate-release.sh, which fails when a tag has no changelog entry. That script is the one both the manual publisher and .github/workflows/release.yml run, so neither path can ship a version the portal cannot name. A Go test cannot do this: the release version exists only as `-ldflags -X main.buildVersion` at link time, so the test binary sees "dev" and has nothing to compare a changelog against. The release workflow used to fall back to `--notes ""` when the entry was missing, which is how v0.7.0 went out silently. Validation now refuses that tag, so the branch is unreachable and is removed instead of left to describe a state that can no longer occur. changelog_test.go gains the one check it can make in-process: each entry's heading names the version its filename claims, which a file copied from the previous release and renamed would otherwise get wrong in a way neither half reveals alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e * Stop showing owner controls on a device shared to you Two defects on the read-only device page, reported together as "the gear is there and it opens someone else's settings". The gear being there is the Bug-1 defect again, in a second place. `openDevice` sets `#dGear.hidden` for a shared device, but `.chip.icon{display:grid}` has specificity (0,2,0) against the UA stylesheet's (0,1,0) `[hidden]{display:none}`, so the attribute lost and the button rendered at 28px wide (40px on phones). Measured 8/8 across both widths, both roles and both entry paths. The global `[hidden]{display:none!important}` added in the first commit already fixes it — which is the argument for having made that rule global rather than scoping it to `.sgroup button`, since the targeted version would have left this one alive. No further CSS change is needed for it. The gear landing on another device is separate and is fixed here. `openDevice` returns early for a shared device, before `loadDeviceSettings`, so the device-settings pane keeps whatever device was loaded last: its name, its alias in an editable field, its Lark and notification cards, and a Remove Device button. Route `#device/<shared>/settings` back to the device page, the way `allowedSet` already routes a non-admin away from Invites, and rewrite the hash so a reload or bookmark lands on the same screen. A shared device has no owner-settings screen to show; the relay 403s every control on it. The mode chip needed no change: it is already `disabled` for shared devices on every path measured, and the read-only banner and fingerprint were present in all 20 direct navigations, including with `/api/devices` delayed. Guard both in tools/portalpreview/shared-device.mjs. The sweep measures whether a screen is laid out correctly; it cannot say whether the screen should exist at all. This asks only the second question, only about shared devices, with boolean criteria and a non-zero exit: gear not visible, mode chip not interactive, banner present, fingerprint filled, settings URL redirected. It walks both entry paths, since the direct-navigation one is where issue #48's race would surface. Verified by reverting each fix in turn: without the CSS rule it reports the visible gear on both widths, without the redirect it reports the settings screen on both widths. The harness gains `?slow=<ms>` on /api/devices, without which no device-list race can be reproduced locally, and `?scene=emptylists`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e * Separate the two device names in tables, and centre empty table rows Two cosmetic defects from user screenshots, both cases of a style being silently defeated by a selector written for something else. The Device column of the sharing table printed "workshopbench-02": the display name and the machine name with nothing between them. `realHTML()` concatenates the second straight onto the first with no whitespace, and each caller is expected to give `.real` a shape of its own — the device card puts it on its own line, the aggregated pending list leaves 6px. The sharing table and the pairing sheet's key-value list were added later and set nothing, so in those two the names ran together. Give both a middle dot, matching the device options in the Share-a-device form, which already read "display name · machine name". Measured: 0px between the two names before, 10px after. The empty-table row was pinned to the right edge. Its own comment says it should read "in the same voice as .blank", and `.blank` is centred — but the row is a colspan cell, which makes it trivially the last child, so `td:last-child{text-align:right}`, written for the actions column, won on specificity. All four tables were affected (tokens, invites, friends, shared devices), so this is not an inconsistency between them but one defect in all of them; the user happened to hit it on the sharing page. Qualify the rule as `td.tempty` so it outranks the actions-column rule. Measured with the four lists emptied: text-align right with 734-760px of dead space on the left, now centred with equal margins on both sides. Not changed: the sharing page's Chinese intro line. It was reported as containing an untranslated "share with friends", but on main it reads "把某台设备的部分权限分享给好友。先在「好友」里加上对方。" with no Latin characters at all, and it read the same at v0.6.1. Nothing to fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e --------- Co-authored-by: 张以琳 <zhangyilin@thunder.com.cn> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Add the v0.7.1 changelog entry: fingerprint-based device promotion, stable macOS device names, shared-device discovery in wanctl peers, clearer relay refusals, and the portal fixes from #59. Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e Co-authored-by: 张以琳 <zhangyilin@thunder.com.cn> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The client half of ADR 0007's per-share switch, on #61's contract: `manage` on shared rows of /api/devices and on every /api/acl row, POST /api/acl/manage to flip one, POST /api/acl to create with it. Device page. `mayAdminister` is the one predicate for who may drive a device's control plane — your own devices always, a shared one only when its grant carries `manage` — replacing the reads of `.shared` that decided the same thing in six places. It mirrors #61's requireDeviceConsole. The guard that was one guard is now two, because the two questions have different answers: `adminGuard` for approvals, pairing, trust, rules and mode; `roGuard` for alias, unbind, Feishu, notify and ADB, which are the owner's rather than the device's. A managed share gets the console the owner sees, with a banner that says whose device it is, in both languages, because the screen underneath is otherwise indistinguishable from your own. The gear stays hidden on every shared device: everything behind it is the owner's, so the switch reaches none of it. Sharing table. The permissions column is gone — every share carries the same set now — and the switch that does vary takes its place, with the same `.sw` control used elsewhere. Turning it on asks first, through the theme sheet: it hands over the approvals, trust, rules and mode of someone else's machine. Turning it off does not ask, because making someone stop and confirm a revocation is how you get people not to revoke. The share form carries the same switch, off by default. `formBox` grew a checkbox field to hold it. Audit tables, both copies: SEC-B-01 and SEC-C-02 are **narrowed** to shares without `manage`, not superseded. There is a read-only share again — it is the default one — so the finding still describes a live state. Also fixes issue #48, which I had wrongly reported as not reproducible. I tested only the entry where the hash is already in the URL at load; that fires no hashchange and never races. The path that races is a hash set *during* load: the event is queued before the listener exists and delivered after it, so `route()` runs before /api/devices resolves. `loadDevices` knew about this and repaired the name and alias field, leaving every access-dependent conclusion at its empty `devMeta` value — a shared device drawn as if it were yours, gear showing, mode chip live, no banner, blank fingerprint, and no recovery, because nothing re-ran. The gating is now one function called from both places, with an idempotent console start so the second pass opens no second poll. Evidence. shared-device.mjs covers both kinds of share across both widths and both entry paths, 22 assertions. Go: a table over the seven mutation routes plus the two owner-only ones, with the switch off and on, 18 subtests — sharedread_test covers the reads, and the routes that change a device were the ones left to inference. Fixtures gain `quarry`, a managed share with something to manage, and a second ACL row so the column shows both states. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on
feat/shared-inherits-owner-rights(#61). Retarget to main once thatmerges.
Fixes #48
Summary
The portal half of ADR 0007's per-share management switch, built on #61's
contract:
manageon shared rows of/api/devicesand on every/api/aclrow,POST /api/acl/manageto flip one,POST /api/aclto create with it. Fieldnames match the contract exactly; nothing needed renaming.
Device page.
mayAdministeris the single predicate for who may drive adevice's control plane — your own devices always, a shared one only when its
grant carries
manage. It replaces reads of.sharedthat were deciding thesame thing in six places, and mirrors #61's
requireDeviceConsole. The guardthat was one guard is now two, because the two questions now have different
answers:
adminGuardcovers approvals, pairing, trust, rules and mode;roGuardstays on alias, unbind, Feishu, notify and ADB, which are the owner'srather than the device's. A managed share gets the console the owner sees, with
a banner naming the owner in both languages, because the screen underneath is
otherwise indistinguishable from your own. The gear stays hidden on every shared
device: everything behind it belongs to the owner, so the switch reaches none of
it, and hiding it removes a screen whose every control would refuse.
Sharing table. The permissions column is gone, since every share now carries
the same set, and the switch that does vary takes its place using the same
.swcontrol as elsewhere. Turning it on asks first, through the theme sheet, because
it hands over the approvals, trust, rules and mode of someone else's machine.
Turning it off does not ask: making someone stop and confirm a revocation is how
you get people not to revoke. The share form carries the same switch, off by
default;
formBoxgrew a checkbox field to hold it.Audit tables, both copies: SEC-B-01 and SEC-C-02 are narrowed rather
than superseded, and SEC-B-01 records the three-way split. I would have left
these rows to #61 as you asked, but its text still says "there is no read-only
share", which the default share now is — so it contradicts what the portal does.
Hand them back to the #61 agent if you would rather; the rows are self-contained
and I will drop my version.
The activity log is a use right
Decided separately, and built here.
wanctl logsalready works for everygrantee, so gating the same log behind the management switch in the portal would
recreate the split ADR 0007 removed: the same person, the same device, allowed
in one client and refused in the other.
requireDeviceUsejoins the other two gates — the owner, or anyone holding alive share, whatever the switch says — and
eventsandlogsmove onto it. Theswitch keeps deciding who may change a device, which is what it was for.
On the page, a read-only share shows the Activity tab and opens on it, because
it is the only tab with anything in it. Waiting and Trust & rules are hidden
rather than shown dead: a tab that opens onto "only the owner can answer this"
is a worse answer than not offering the tab.
Three route kinds, three answers:
sharedread_test.goasserted the opposite foreventsandlogs, so its firstcase drops them and states why, and the other half of the decision gets its own
case.
Also fixes #48
I reported earlier that #48 was not reproducible. That was wrong, and this
fixes it. I had tested only the entry where the hash is already in the URL at
load; that fires no
hashchangeand never races. The path that races is a hashset during load: the event is queued before the listener exists and delivered
after it, so
route()runs before/api/devicesresolves.loadDevicesalready knew about this and repaired the device name and the aliasfield, and left every access-dependent conclusion at its empty-
devMetavalue.A shared device drew as if it were yours: gear showing, mode chip live, no
banner, blank fingerprint. It never healed, because nothing re-ran. Measured
before and after at the same URL:
hiddenThe gating is now one function called both when the page opens and again when
the device list lands, with an idempotent console start so the second pass opens
no second poll. Verified load-bearing by removing the repair again: six
assertions fail across both widths.
Test plan
gofmt -l .clean,go vet ./...andgo vet -tags lark ./...clean,go test ./...andgo test -tags lark ./...pass.Go — the mutation routes
sharedread_test.goin #61 covers five read routes. The routes that change adevice were left to inference, and they are the ones worth getting wrong. Added
a table over the seven control-plane mutations plus the two owner-only ones,
with the switch off and on: 18 subtests.
Each refusal also asserts the relay was never reached — a gate that answers 403
after already fetching the owner's console has still read it. Verified
load-bearing by making
requireDeviceConsoleignoreaccess.Manage: everymanage=truecontrol-plane row fails, owner-only rows keep passing.Browser — shared devices
tools/portalpreview/shared-device.mjs, both kinds of share, both widths, bothentry paths:
Verified load-bearing three ways: removing the
[hidden]rule surfaces thegear, removing the settings redirect surfaces the settings screen, and leaving
the two tabs visible for a read-only share produces six failures across both
widths.
Browser — the Manage column
Driven through the real handlers and the theme sheet, at 1200 and 390, in both
languages:
manage:falserow renders off,aria-pressed="false"manage:truerow renders on,aria-pressed="true"POST /api/acl/manage {"device":"bench-02","grantee":"rowan","manage":true}permskeyFixtures gain
quarry, a managed share with a pending approval, a pendingpairing, a rule and a trusted controller, because a state that renders
identically to another is not one the harness can see; a second ACL row so the
column shows both states; and a real write for
/api/acl/manage, so the switchdoes not spring back after a successful flip.
Two things for you
This branch carries a merge of main. #61 is two commits behind main and does
not contain #59, including
[hidden]{display:none!important}— which is exactlywhat keeps the gear hidden on a shared device, so without it the shared-device
guard fails on the assertion it exists for. I merged
origin/mainin rather thanregress that. The consequence is that the diff against #61's branch also shows
#59 and #62. Rebasing #61 onto main would clear it; I did not touch someone
else's branch.
#61 puts
/api/devices/adb-pairbehindrequireDeviceConsole, so a granteewith
managemay pair ADB over the API. The portal never offers it to them: theADB card lives behind the gear, which is hidden on every shared device. Not a
hole — the server check is the real one — but the UI and the API disagree about
who may do it, and one of the two should move. My reading is that ADB pairing is
device setup rather than device operation and belongs with the owner, but it is
#61's call.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SpPAyRQzTAw7MbmJqZdm7e