mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(sync): a refused push stays refused, and says why (BEA-403) (#146)
A device whose journal push the hub 403s reported healthy sync between every pair of remote passes, and never showed the hub's reason for the refusal — so a user whose device was not registered to their account re-ran `bdrive login` (which the message told them to), re-checked their project permissions (write), and had nowhere left to look. Two causes, both local to the client: - Cycle recomputed st.Access from scratch at the end of every pass, including the daemon's cheap local-only ticks that never reach the hub. Three of those run between remote passes, so the daemon alternated "read-only on this project" / "access restored; syncing normally" every few seconds and `bdrive status` reported OK moments after a refused push. Now each leg records its own verdict — pull clears no-access, push records read-only or clears it — and a cycle that asked nothing leaves the last answer standing. - The hub's own sentence was summarized into "read-only (pull only)", which describes the STATUS CODE. It is the only thing that tells a device-registration refusal from a project the user really is a reader on. It now rides in SyncState.AccessReason and Result.Reason(), printed by `bdrive sync`, `bdrive status` and the daemon log, and dropped unless it passes journal.SafeText — hub text reaching a terminal. The hub's refusal also now names the upgrade: the binding is made by the login request naming its device, which a CLI older than the gate does not do, so "run `bdrive login`" alone sent that user in a circle. Hub and CLI deploy separately, so the skew is the expected state right after the gate ships. Claude-Session: https://claude.ai/code/session_01GSHsQU4pBCzKkPyPeXSwTm Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
709c40e76c
commit
922886b949
@@ -397,6 +397,14 @@ as being offline (see `bdrive status`):
|
||||
pulled, pushed, or written: revoking access never deletes or reverts a
|
||||
file on someone's disk. Re-granting resumes on the next tick.
|
||||
|
||||
`bdrive status` and `bdrive sync` print the hub's own sentence under either
|
||||
one, as `reason:`. Read it: not every refused push is a permissions
|
||||
question. `this device is not registered to your account on this hub` means
|
||||
this machine's device identity was never bound to your account — update
|
||||
`bdrive` and run `bdrive login` here. Project settings will show `write` and
|
||||
explain nothing. Both states are recorded only by a cycle that actually
|
||||
reached the hub, so the local-only ticks in between never revise the answer.
|
||||
|
||||
Public `/s/<token>` share links are **unaffected** by any of this: they are
|
||||
anonymous by design and keep serving until revoked, so cutting someone's
|
||||
access does not kill links they already minted.
|
||||
|
||||
@@ -37,8 +37,10 @@ classDiagram
|
||||
+Conflicts +Pruned +Materialized
|
||||
+Pushed +Offline +OfflineErr
|
||||
+ReadOnly +NoAccess +AccessErr
|
||||
+Reason() string
|
||||
}
|
||||
note for Result "Offline / ReadOnly / NoAccess are three different answers: unreachable (retry all), push refused (pull-only), pull refused (pause, touch nothing)"
|
||||
note for Result "Reason() is accessReason(AccessErr): the hub's own sentence for a refusal, minus the wrapper chain, dropped unless it passes journal.SafeText. 'read-only' summarizes the STATUS CODE — the sentence is the only thing that tells a device-registration 403 from a project the user really is a reader on"
|
||||
|
||||
class Filter {
|
||||
+Skip(rel) bool
|
||||
@@ -52,8 +54,10 @@ classDiagram
|
||||
|
||||
class SyncState {
|
||||
+Lamport +PushedOps +Access
|
||||
+AccessReason
|
||||
+IgnoreAccepted +IgnorePulled
|
||||
}
|
||||
note for SyncState "Access/AccessReason are written ONLY by the leg that asked the hub — pull clears no-access, push records read-only or clears it. A cycle with no remote leg leaves the last answer standing, so the daemon's local-only ticks stop alternating 'read-only' / 'access restored' and bdrive status stops reporting healthy sync moments after a refused push"
|
||||
note for SyncState "store — IgnoreAccepted is the .bdriveignore scope THIS device consented to; IgnorePulled is the text a peer's version last wrote here. The pair is what tells a locally authored rule change from one that arrived over the wire. vouchedFloor seeds it once on upgrade: keep every exclusion, drop each `!` that no already-materialized path vouches for"
|
||||
|
||||
class SafePath {
|
||||
|
||||
@@ -255,6 +255,12 @@ func statusCmd() *cobra.Command {
|
||||
case store.AccessNone:
|
||||
fmt.Printf(" access: no access to this project — sync paused\n")
|
||||
}
|
||||
// `status` is the command someone runs when sync is stuck, and
|
||||
// it never talks to the hub — so the refusal it reports is only
|
||||
// as useful as the reason the last cycle recorded with it.
|
||||
if st.AccessReason != "" {
|
||||
fmt.Printf(" reason: %s\n", safeField(st.AccessReason, 300))
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -179,6 +179,16 @@ func humanBytes(n int64) string {
|
||||
return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "KMGTPE"[exp])
|
||||
}
|
||||
|
||||
// printReason prints the hub's stated reason under a refusal line. "read-only"
|
||||
// is a summary of the STATUS code; the sentence under it is the only thing that
|
||||
// tells a device-registration 403 apart from a project the user really is a
|
||||
// reader on — and only one of those is fixed by signing in again.
|
||||
func printReason(reason string) {
|
||||
if reason != "" {
|
||||
fmt.Printf(" %s\n", safeField(reason, 300))
|
||||
}
|
||||
}
|
||||
|
||||
func printCycle(res *syncer.Result) {
|
||||
fmt.Printf(" local changes: %d\n", res.LocalOps)
|
||||
fmt.Printf(" pulled changes: %d\n", res.PulledOps)
|
||||
@@ -192,8 +202,10 @@ func printCycle(res *syncer.Result) {
|
||||
switch {
|
||||
case res.NoAccess:
|
||||
fmt.Printf(" remote: no access — sync paused (ask a project admin for access)\n")
|
||||
printReason(res.Reason())
|
||||
case res.ReadOnly:
|
||||
fmt.Printf(" remote: read-only (pull only) — local changes stay on this device\n")
|
||||
printReason(res.Reason())
|
||||
case res.Pushed && res.OfflineErr != nil:
|
||||
// Offline is a report, not a gate (see syncer.Result): a content-level
|
||||
// problem with one object no longer withholds this device's push, so
|
||||
|
||||
@@ -449,7 +449,11 @@ func Run(folder string, scanInterval, remoteInterval time.Duration) error {
|
||||
lastRemote = time.Now()
|
||||
case res.ReadOnly:
|
||||
if lastAccess != store.AccessReadOnly {
|
||||
log.Printf("read-only on this project, pulling only; local changes stay on this device")
|
||||
if reason := res.Reason(); reason != "" {
|
||||
log.Printf("read-only on this project, pulling only; local changes stay on this device (%s)", reason)
|
||||
} else {
|
||||
log.Printf("read-only on this project, pulling only; local changes stay on this device")
|
||||
}
|
||||
lastAccess = store.AccessReadOnly
|
||||
}
|
||||
lastRemote = time.Now()
|
||||
@@ -461,7 +465,12 @@ func Run(folder string, scanInterval, remoteInterval time.Duration) error {
|
||||
}
|
||||
lastRemote = time.Now()
|
||||
default:
|
||||
if lastAccess != store.AccessOK {
|
||||
// "access restored" is a claim about the hub, and a local-only tick
|
||||
// never asked it one. Announcing it there is what made a refused
|
||||
// device alternate between this line and the read-only one every few
|
||||
// seconds: three cheap scan ticks run between remote passes, each
|
||||
// cleared the flag, and the next remote pass set it again.
|
||||
if doRemote && lastAccess != store.AccessOK {
|
||||
log.Printf("access restored; syncing normally")
|
||||
lastAccess = store.AccessOK
|
||||
}
|
||||
|
||||
@@ -246,6 +246,12 @@ type SyncState struct {
|
||||
Lamport int64 `json:"lamport"`
|
||||
PushedOps int64 `json:"pushed_ops"` // how many of our own ops the remote has
|
||||
Access string `json:"access,omitempty"` // "", "read-only", or "no-access"
|
||||
// AccessReason is the hub's own words for the last refusal. Without it every
|
||||
// 403 renders as the same "read-only (pull only)" line, and the hub's most
|
||||
// actionable answer — "this device is not registered to your account on this
|
||||
// hub; run `bdrive login`" — reached nobody: the one state a user cannot
|
||||
// diagnose from the outside was the one the CLI summarized away.
|
||||
AccessReason string `json:"access_reason,omitempty"`
|
||||
|
||||
// IgnoreAccepted is the .bdriveignore text whose scan scope THIS device has
|
||||
// accepted, and IgnorePulled is the text a peer's version last wrote here.
|
||||
|
||||
@@ -127,6 +127,38 @@ type Result struct {
|
||||
AccessErr error
|
||||
}
|
||||
|
||||
// accessReason renders a hub refusal as the sentence a person can act on. The
|
||||
// hub answers a device-registration 403 with what to DO about it, and every
|
||||
// caller used to collapse that into "read-only (pull only)" — the one refusal
|
||||
// that is not about project permissions at all, reported as if it were, so the
|
||||
// user re-checked their access, saw `write`, and had nowhere left to look.
|
||||
//
|
||||
// The wrapper chain the CLI itself added ("forbidden: server: 403 Forbidden: ")
|
||||
// is dropped; it tells a reader nothing the line does not already say.
|
||||
//
|
||||
// The remainder is the HUB's text landing in a log file, a terminal and an
|
||||
// agent's context, so it passes journal.SafeText — the rule this repo already
|
||||
// applies to every other peer-written string it renders — or it does not travel
|
||||
// at all. Bounded for the same reason: this is persisted in sync.json and
|
||||
// printed on one status line.
|
||||
func accessReason(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
msg := err.Error()
|
||||
if _, rest, ok := strings.Cut(msg, "Forbidden: "); ok && rest != "" {
|
||||
msg = rest
|
||||
}
|
||||
if len([]rune(msg)) > 300 || !journal.SafeText(msg) {
|
||||
return ""
|
||||
}
|
||||
return msg
|
||||
}
|
||||
|
||||
// Reason is the hub's own words for a ReadOnly or NoAccess answer, empty when
|
||||
// the hub gave none. It is what the CLI prints under the summary line.
|
||||
func (r *Result) Reason() string { return accessReason(r.AccessErr) }
|
||||
|
||||
func (r *Result) Activity() bool {
|
||||
return r.LocalOps > 0 || r.PulledOps > 0 || r.Conflicts > 0 || r.Pruned > 0 || r.Materialized > 0
|
||||
}
|
||||
@@ -254,13 +286,18 @@ func (s *Session) Cycle(ctx context.Context) (*Result, error) {
|
||||
pulled, gone, err = s.pull(ctx)
|
||||
switch {
|
||||
case err == nil:
|
||||
if st.Access == store.AccessNone {
|
||||
// The pull that was refused now succeeds, so read access is back.
|
||||
// Whether writes are back is the push leg's to answer below.
|
||||
st.Access, st.AccessReason = store.AccessOK, ""
|
||||
}
|
||||
case errors.Is(err, remote.ErrForbidden):
|
||||
// Access to this project was revoked. Stop here: materializing a
|
||||
// replay we can no longer refresh would look like the hub
|
||||
// reverting the user's files. Nothing is pushed, nothing is
|
||||
// deleted, and the next cycle re-checks.
|
||||
res.NoAccess, res.AccessErr = true, err
|
||||
st.Access = store.AccessNone
|
||||
st.Access, st.AccessReason = store.AccessNone, accessReason(err)
|
||||
return res, s.finish(cache, st)
|
||||
case errors.Is(err, errBlobContent):
|
||||
// Reported — it is the only signal a device ever gets that its hub
|
||||
@@ -422,6 +459,7 @@ func (s *Session) Cycle(ctx context.Context) (*Result, error) {
|
||||
switch err := s.push(ctx, myOps, &st); {
|
||||
case err == nil:
|
||||
res.Pushed = true
|
||||
st.Access, st.AccessReason = store.AccessOK, ""
|
||||
case errors.Is(err, remote.ErrForbidden):
|
||||
// Read-only on this project: pull and materialize already ran, so
|
||||
// pull-only is the steady state. Our own ops stay in the local
|
||||
@@ -429,6 +467,7 @@ func (s *Session) Cycle(ctx context.Context) (*Result, error) {
|
||||
// attempted once per remote interval (no hot loop, and a re-grant
|
||||
// self-heals).
|
||||
res.ReadOnly, res.AccessErr = true, err
|
||||
st.Access, st.AccessReason = store.AccessReadOnly, accessReason(err)
|
||||
default:
|
||||
res.Offline = true
|
||||
res.OfflineErr = err
|
||||
@@ -451,10 +490,14 @@ func (s *Session) Cycle(ctx context.Context) (*Result, error) {
|
||||
}
|
||||
}
|
||||
|
||||
st.Access = store.AccessOK
|
||||
if res.ReadOnly {
|
||||
st.Access = store.AccessReadOnly
|
||||
}
|
||||
// st.Access is NOT recomputed here. Only the leg that actually asked the hub
|
||||
// knows how it answered, so each one records its own verdict above and a
|
||||
// cycle that never asked leaves the last one standing. Resetting it to OK on
|
||||
// every cycle meant the daemon's cheap local-only ticks — three of them
|
||||
// between remote passes — each declared "access restored; syncing normally",
|
||||
// so a device the hub was refusing alternated between the two log lines
|
||||
// forever and `bdrive status` reported healthy sync moments after the push
|
||||
// it had just refused.
|
||||
if err := s.finish(cache, st); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -491,6 +491,98 @@ func TestReadOnlyDevicePullsOnly(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// refusingPush is a hub that answers every push with a real 403 body. The one
|
||||
// that matters is the device-registration refusal: it is not about project
|
||||
// permissions at all, so the user who checks their permissions finds `write`
|
||||
// and has nowhere left to look — the hub's sentence is the only thing that
|
||||
// points at the fix.
|
||||
type refusingPush struct {
|
||||
remote.Backend
|
||||
msg string
|
||||
}
|
||||
|
||||
func (p refusingPush) Put(ctx context.Context, key string, r io.Reader, size int64) error {
|
||||
return fmt.Errorf("%w: server: 403 Forbidden: %s", remote.ErrForbidden, p.msg)
|
||||
}
|
||||
|
||||
// A refused device must stay refused in its own records until the hub says
|
||||
// otherwise — and it must be able to say WHY.
|
||||
//
|
||||
// Both halves come from one report: a mount whose journal pushes 403'd on every
|
||||
// remote pass, while `bdrive status` said access was fine and the daemon log
|
||||
// alternated "read-only on this project" / "access restored; syncing normally"
|
||||
// every few seconds. The cycle recomputed access from scratch at the end of
|
||||
// every pass, including the cheap local-only ones the daemon runs three of
|
||||
// between remote passes, so the hub's answer was overwritten by a cycle that
|
||||
// never asked it anything. And the answer itself — "this device is not
|
||||
// registered to your account on this hub; run `bdrive login`" — was summarized
|
||||
// into "read-only (pull only)" and lost.
|
||||
func TestRefusedPushKeepsItsVerdictAndItsReason(t *testing.T) {
|
||||
const refusal = "this device is not registered to your account on this hub; run `bdrive login` on this machine"
|
||||
be := sharedRemote(t)
|
||||
b := newDevice(t, "devb", refusingPush{Backend: be, msg: refusal})
|
||||
|
||||
write(t, b.Folder, "mine.md", "local only")
|
||||
res, err := b.Cycle(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !res.ReadOnly {
|
||||
t.Fatalf("a refused push should report ReadOnly: %+v", res)
|
||||
}
|
||||
if res.Reason() != refusal {
|
||||
t.Fatalf("Result.Reason() = %q, want the hub's own sentence %q", res.Reason(), refusal)
|
||||
}
|
||||
st, err := b.Store.LoadSync()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if st.Access != store.AccessReadOnly || st.AccessReason != refusal {
|
||||
t.Fatalf("persisted access = %q/%q, want read-only + the hub's reason", st.Access, st.AccessReason)
|
||||
}
|
||||
|
||||
// The daemon's local-only tick: same session, no backend at all. It learns
|
||||
// nothing about the hub, so it must not clear the hub's last answer.
|
||||
b.Backend = nil
|
||||
write(t, b.Folder, "second.md", "still local")
|
||||
if res := cycle(t, b); res.ReadOnly {
|
||||
t.Fatalf("a cycle with no remote leg cannot discover a refusal: %+v", res)
|
||||
}
|
||||
st, _ = b.Store.LoadSync()
|
||||
if st.Access != store.AccessReadOnly || st.AccessReason != refusal {
|
||||
t.Fatalf("a local-only tick reset access to %q/%q — `bdrive status` reports healthy "+
|
||||
"sync moments after the push the hub refused, and the daemon logs "+
|
||||
"\"access restored\" between every pair of remote passes", st.Access, st.AccessReason)
|
||||
}
|
||||
|
||||
// Only the hub clears it: a push that lands does, and takes the stale
|
||||
// reason with it.
|
||||
b.Backend = be
|
||||
if res := cycle(t, b); !res.Pushed {
|
||||
t.Fatalf("re-granted device did not push: %+v", res)
|
||||
}
|
||||
if st, _ := b.Store.LoadSync(); st.Access != store.AccessOK || st.AccessReason != "" {
|
||||
t.Fatalf("after a successful push, access = %q/%q, want cleared", st.Access, st.AccessReason)
|
||||
}
|
||||
}
|
||||
|
||||
// A hub message that could repaint the terminal it lands in is dropped rather
|
||||
// than rendered: it reaches a log file, `bdrive status`, and an agent's context
|
||||
// verbatim, and the hub is the one string source here nobody local vouches for.
|
||||
func TestAccessReasonRefusesTerminalControls(t *testing.T) {
|
||||
esc := fmt.Errorf("%w: server: 403 Forbidden: nope\x1b[2Kaccess restored", remote.ErrForbidden)
|
||||
if got := accessReason(esc); got != "" {
|
||||
t.Errorf("accessReason kept a control sequence: %q", got)
|
||||
}
|
||||
long := fmt.Errorf("%w: server: 403 Forbidden: %s", remote.ErrForbidden, strings.Repeat("x", 400))
|
||||
if got := accessReason(long); got != "" {
|
||||
t.Errorf("accessReason kept a %d-rune message", len([]rune(got)))
|
||||
}
|
||||
if got := accessReason(nil); got != "" {
|
||||
t.Errorf("accessReason(nil) = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A device whose access is revoked entirely pauses: the cycle reports
|
||||
// NoAccess (not Offline), the working folder is left byte-for-byte alone —
|
||||
// revoking access must never look like the hub deleting someone's files — and
|
||||
|
||||
@@ -133,9 +133,17 @@ func (s *Server) ownJournal(w http.ResponseWriter, r *http.Request, key string)
|
||||
// machine's token (DeviceRegistry.Bind), which is a moment the hub
|
||||
// authenticates and the machine cannot forge. So an unowned id is
|
||||
// simply not anybody's to write, and the remedy is to sign in.
|
||||
// "run `bdrive login`" alone sent one user in a circle for an
|
||||
// afternoon: the binding is made by the login request naming its
|
||||
// device, which a CLI older than this gate does not do, so signing in
|
||||
// again succeeded, changed nothing, and every push kept 403ing with
|
||||
// the same sentence. The hub and the CLI deploy separately, so the
|
||||
// skew is the expected state right after this ships — the refusal has
|
||||
// to name the upgrade, not just the command.
|
||||
http.Error(w, "this device is not registered to your account on this hub; "+
|
||||
"run `bdrive login` on this machine (if the id belongs to someone else, "+
|
||||
"delete device.json in your BearDrive home first, or ask a project admin)",
|
||||
"update bdrive, then run `bdrive login` on this machine (an older CLI signs in "+
|
||||
"without registering its device). If the id belongs to someone else, delete "+
|
||||
"device.json in your BearDrive home first, or ask a project admin",
|
||||
http.StatusForbidden)
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -206,19 +206,30 @@ touches your files:
|
||||
```
|
||||
pending: 3 local change(s) not yet pushed
|
||||
access: read-only (pull only) — 3 local change(s) stay on this device
|
||||
reason: this device is not registered to your account on this hub; update bdrive, then run `bdrive login` on this machine
|
||||
```
|
||||
|
||||
- **`read-only (pull only)`** — you have `read` on the project. The daemon keeps
|
||||
pulling teammates' changes; your own edits stay journaled locally, never
|
||||
pushed and never dropped. They go out if you are granted `write` again.
|
||||
- **`read-only (pull only)`** — the hub refused this device's push. Usually you
|
||||
have `read` on the project: the daemon keeps pulling teammates' changes, your
|
||||
own edits stay journaled locally, never pushed and never dropped, and they go
|
||||
out if you are granted `write` again.
|
||||
- **`no access to this project — sync paused`** — your access was revoked.
|
||||
Nothing is pulled, pushed, or written; the working folder is left exactly as
|
||||
it is. Re-granting resumes on the next tick with no manual step.
|
||||
|
||||
Always read the `reason:` line under them — it is the hub's own sentence, and
|
||||
not every refusal is about project permissions. The common non-permission one
|
||||
is `this device is not registered to your account on this hub`: your device
|
||||
identity was never bound to your account, which is fixed by updating `bdrive`
|
||||
and running `bdrive login` on that machine, not in Project settings. Checking
|
||||
your permissions there will show `write` and tell you nothing.
|
||||
|
||||
`bdrive sync` shows the same two as `remote: read-only (pull only)` /
|
||||
`remote: no access — sync paused`, and the daemon logs each once on
|
||||
transition rather than on every tick. Both are permission answers: they are
|
||||
fixed in the hub's Project settings → People, not on the device. See
|
||||
`remote: no access — sync paused` with the reason on the line below, and the
|
||||
daemon logs each once on transition rather than on every tick — including the
|
||||
cheap local-only ticks between remote passes, which never ask the hub anything
|
||||
and so never revise its last answer. For the permission answers, the fix is in
|
||||
the hub's Project settings → People; see
|
||||
[Project permissions](/concepts/permissions/).
|
||||
|
||||
### `bdrive login` and switching hubs
|
||||
|
||||
Reference in New Issue
Block a user