tfks/dropbear
tfks/dropbear
4
0
Fork 0
Code Issues 5 Pull requests Projects Releases 3 Packages Wiki Activity Actions

Allow device to register with remote info in its .dropbear/root.toml #14

New issue
Closed
opened 2026-06-04 08:50:00 +00:00 by erik · 7 comments
erik commented 2026-06-04 08:50:00 +00:00
Owner
Copy link

To make life simpler for new device registrations, root.toml files could be shared, so when the target dir contains .dropbear/root.toml already, then use its values as defaults for the register --remote-* cli args.

To make life simpler for new device registrations, root.toml files could be shared, so when the target dir contains .dropbear/root.toml already, then use its values as defaults for the `register --remote-*` cli args.
agent-pm commented 2026-06-04 23:38:40 +00:00
Member
Copy link

Brief — QA

Phase: default. Run fjx qa prompt default for the cycle protocol.

PR: #17.

Context

  • The diff is wider than both the issue text and the PR #17 body. The issue/PR describe only inheriting [remote] defaults from an existing root.toml for register --remote-*. Commit 45d9860 also relaxes config.Init: it now overwrites a matching-root_id root.toml when the requested device_id differs from the on-disk one (genuine new-device registration without --force); same device_id still requires --force; a differing root_id is still refused as a relabel. Review and adversarially probe this guard logic — it is the higher-risk half and is not covered by the PR description.
  • The just validate test::audit (govulncheck) gate fails on 3 Go stdlib vulns (GO-2026-5036/5037/5038, fixed in go1.26.4; toolchain go1.26.3) in unrelated files. This reproduces on clean origin/main and is environmental — do not send back to dev for it.
<!-- pm:brief:qa --> # Brief — QA **Phase:** default. Run `fjx qa prompt default` for the cycle protocol. PR: #17. ## Context - The diff is wider than both the issue text and the PR #17 body. The issue/PR describe only inheriting `[remote]` defaults from an existing `root.toml` for `register --remote-*`. Commit 45d9860 also relaxes `config.Init`: it now overwrites a matching-`root_id` root.toml when the requested `device_id` differs from the on-disk one (genuine new-device registration without `--force`); same `device_id` still requires `--force`; a differing `root_id` is still refused as a relabel. Review and adversarially probe this guard logic — it is the higher-risk half and is not covered by the PR description. - The `just validate` `test::audit` (govulncheck) gate fails on 3 Go stdlib vulns (GO-2026-5036/5037/5038, fixed in go1.26.4; toolchain go1.26.3) in unrelated files. This reproduces on clean `origin/main` and is environmental — do not send back to dev for it.
agent-pm commented 2026-06-04 23:38:40 +00:00
Member
Copy link

Ledger for: agent-pm

Status: closed @ 2026-06-26T23:35:31.672Z
Briefs:

Activity

  1. i14/closed : applied

(PM manages this ledger; agent section unused)

<!-- agent-pm:ledger --> ## Ledger for: agent-pm Status: closed @ 2026-06-26T23:35:31.672Z Briefs: - [dev](#issuecomment-3101) - [qa](#issuecomment-2575) ### Activity 1. i14/closed : applied --- _(PM manages this ledger; agent section unused)_
agent-dev commented 2026-06-07 00:10:12 +00:00
Member
Copy link

Ledger for: agent-dev

Status: review @ 2026-06-26T22:39:05.850Z
Refs: #17

Rework — PR #17 (mode inheritance)

Scope: the single fix $FJX_OWNER scoped from the QA ledger — inherit mode from an existing .dropbear/root.toml, symmetrically with the [remote] defaults. The two explicitly out-of-scope QA items (created_at re-stamp, same-host typo overwrite / no-force guard) were left untouched.

Fix — commit eeff880

  • --mode flag default changed from "bidirectional" to "" (help text updated to note the inherited default).
  • registerExecute now resolves mode the same way [remote] is resolved: explicit --mode wins; else inherit from the on-disk root.toml; else fall back to "bidirectional".
  • Added loadModeDefault(path) mirroring loadRemoteDefaults — reads mode from an existing .dropbear/root.toml, returns "" when absent/unreadable/invalid.
  • A device that omits --mode against a shared upload-only root now keeps upload-only instead of silently downgrading to bidirectional.

Test — TestRegisterInheritsModeFromExistingRootTOML

Table-driven, two subtests against a seeded mode = "upload-only" root:

  • --mode omitted → written root.toml keeps upload-only (the previously-untested inheritance path).
  • explicit --mode download-only → overrides to download-only.

Both pass; full suite go test ./... → 772 passed.

Validation

just check (format/vet/lint) clean. just validate's only failure is test::audit (govulncheck GO-2026-5037/5038/5039), which is environmental: toolchain is go1.26.3, all three are fixed in go1.26.4, and every reported trace is in pre-existing code (deviceregistry/objectstore/statusapi/main) untouched by this diff. This diff adds no dependencies. Matches the env-failure QA already recorded on this PR.

<!-- agent-dev:ledger --> ## Ledger for: agent-dev Status: review @ 2026-06-26T22:39:05.850Z Refs: #17 --- ## Rework — PR #17 (mode inheritance) **Scope:** the single fix `$FJX_OWNER` scoped from the QA ledger — inherit `mode` from an existing `.dropbear/root.toml`, symmetrically with the `[remote]` defaults. The two explicitly out-of-scope QA items (`created_at` re-stamp, same-host typo overwrite / no-force guard) were left untouched. ### Fix — commit `eeff880` - `--mode` flag default changed from `"bidirectional"` to `""` (help text updated to note the inherited default). - `registerExecute` now resolves mode the same way `[remote]` is resolved: explicit `--mode` wins; else inherit from the on-disk `root.toml`; else fall back to `"bidirectional"`. - Added `loadModeDefault(path)` mirroring `loadRemoteDefaults` — reads `mode` from an existing `.dropbear/root.toml`, returns `""` when absent/unreadable/invalid. - A device that omits `--mode` against a shared `upload-only` root now keeps `upload-only` instead of silently downgrading to `bidirectional`. ### Test — `TestRegisterInheritsModeFromExistingRootTOML` Table-driven, two subtests against a seeded `mode = "upload-only"` root: - `--mode` omitted → written `root.toml` keeps `upload-only` (the previously-untested inheritance path). - explicit `--mode download-only` → overrides to `download-only`. Both pass; full suite `go test ./...` → 772 passed. ### Validation `just check` (format/vet/lint) clean. `just validate`'s only failure is `test::audit` (govulncheck GO-2026-5037/5038/5039), which is environmental: toolchain is go1.26.3, all three are fixed in go1.26.4, and every reported trace is in pre-existing code (deviceregistry/objectstore/statusapi/main) untouched by this diff. This diff adds no dependencies. Matches the env-failure QA already recorded on this PR.
erik commented 2026-06-25 04:47:16 +00:00
Author
Owner
Copy link

@agent-pm tooling issues should be fixed. Have agent-dev try again.

@agent-pm tooling issues should be fixed. Have agent-dev try again.
agent-dev removed their assignment 2026-06-26 20:57:05 +00:00
agent-qa commented 2026-06-26 21:44:07 +00:00
Member
Copy link

Ledger for: agent-qa

Status: concerns @ 2026-06-26T22:17:01.863Z

QA review — PR #17 (commit 45d9860)

Verdict: concerns — relaxing config.Init's no-force guard widens the silent-clobber surface in a way the PR description, issue, and tests don't cover. Nothing is outright broken; the guard logic is sound, but a same-host typo now succeeds silently where it used to error, and silently resets mode.

Action summary

  • validate.yaml run #29 on head 45d9860: success (the only workflow on the PR head). No failed required checks.
  • test::audit (govulncheck) failure noted in the brief is environmental (GO-2026-5036/5037/5038, fixed in go1.26.4; toolchain go1.26.3) and reproduces on clean origin/main — not attributable to this diff. Not sent back for it.
  • Local go build ./... clean; go test ./internal/config/ ./cmd/dropbear/ → 169 passed.

What I verified (guard is sound where it counts)

  • requestedDeviceID is user-supplied and required (register.go:70), not auto-generated, so the new result.ObservedDeviceID != requestedDeviceID clause is not trivially defeated by per-invocation randomness. Good — that was my first hypothesis and it's falsified.
  • In StateOnline, root.toml parsed cleanly, so ObservedDeviceID is always a non-empty, pattern-valid id (validate.go:125-160, roottoml.go:125). The "empty observed device_id always overwrites" edge is unreachable — a missing/invalid device_id yields StateIdentityCorrupt, not StateOnline. Good.
  • The new no-force branch is reachable (not dead code): EnsureRootNotInitialized returns nil when a registry already exists (registry.go:277-297), so the copy-root.toml-to-a-fresh-dir / join-existing-root workflow flows through to Init and exercises the new path.

Confirmed issue — silent, mode-resetting clobber on a same-host re-register (medium)

Pre-PR, every no-force re-init against an existing matching-root_id root.toml was refused with INIT_ALREADY_INITIALIZED. Post-PR, any differing device_id overwrites silently with exit 0. Concretely, a mistyped --device-id on the same machine (e.g. laptop → laptpo) now:

  1. Silently overwrites the local root.toml (reproduced with a throwaway config test, since removed: differing device_id, force=false, Init returns nil and the file is rewritten).
  2. Resets mode: register.go inherits [remote] from the existing root.toml via loadRemoteDefaults, but mode is not inherited — it falls back to the flag default bidirectional (register.go:46). A new/typo device that omits --mode silently downgrades a shared upload-only root to bidirectional. Verified: mode "upload-only" -> "bidirectional" with no warning.
  3. Registers the typo'd id as a phantom device in the shared remote registry — deviceregistry.Register(f.deviceID) runs at register.go:149, before Init. This phantom write already happened pre-PR too, but pre-PR the command then exited non-zero (INIT_ALREADY_INITIALIZED), giving the user a signal. Post-PR the command exits 0, so the only signal that a re-register went wrong is now gone.

Net: the safety net that caught all accidental re-inits now catches only the exact-same-device_id case, and the new path silently rewrites mode.

(Note: root.toml's created_at is also re-stamped on overwrite, but that field is write-only — never read for any decision, never surfaced in output, and stamped with each device's local time.Now() on every write since the schema's inception, so it has never represented a root-level creation time. Re-stamping it is harmless; not a finding.)

Gap analysis (what CI/tests didn't cover)

  • TestInit_newDeviceSameRootNoForce only asserts the happy path (new device succeeds, device_id updated). It does not assert what happens to mode on overwrite, so the silent mode-reset is untested.
  • No test covers the asymmetry between [remote] inheritance and mode non-inheritance in the register command.
  • No test covers the typo-on-same-host case that the relaxation newly admits.

Untested assumption / open question for dev

  • Is silently picking bidirectional for a device that omits --mode against a shared non-bidirectional root intended? If so, a one-line note in the register help / brief would close the gap. If not, mode should be inherited from the existing root.toml symmetrically with [remote].

Recommendation

Send back to dev to either (a) inherit mode alongside [remote] on register, or (b) consciously document the new silent-overwrite semantics (mode reset on differing device_id without --force) in the PR body and register help, and add a test pinning the chosen behavior.

Routing note (fjx)

fjx qa exposes no hand-off/assign verb (only next, ledger, checks, release, prompt); fjx issue is list/view/label only — there is no fjx qa done mirroring fjx dev done. I'm therefore signalling the verdict via this ledger + the existing agent/review label rather than reassigning the issue myself. If QA is expected to reassign to agent-dev on a concerns verdict, that capability is missing at the CLI.

<!-- agent-qa:ledger --> ## Ledger for: agent-qa Status: concerns @ 2026-06-26T22:17:01.863Z --- ## QA review — PR #17 (commit 45d9860) **Verdict: concerns** — relaxing `config.Init`'s no-force guard widens the silent-clobber surface in a way the PR description, issue, and tests don't cover. Nothing is outright broken; the guard logic is sound, but a same-host typo now succeeds silently where it used to error, and silently resets `mode`. ### Action summary - `validate.yaml` run #29 on head `45d9860`: **success** (the only workflow on the PR head). No failed required checks. - `test::audit` (govulncheck) failure noted in the brief is environmental (GO-2026-5036/5037/5038, fixed in go1.26.4; toolchain go1.26.3) and reproduces on clean `origin/main` — not attributable to this diff. Not sent back for it. - Local `go build ./...` clean; `go test ./internal/config/ ./cmd/dropbear/` → 169 passed. ### What I verified (guard is sound where it counts) - `requestedDeviceID` is **user-supplied and required** (`register.go:70`), not auto-generated, so the new `result.ObservedDeviceID != requestedDeviceID` clause is **not** trivially defeated by per-invocation randomness. Good — that was my first hypothesis and it's falsified. - In `StateOnline`, `root.toml` parsed cleanly, so `ObservedDeviceID` is always a non-empty, pattern-valid id (`validate.go:125-160`, `roottoml.go:125`). The "empty observed device_id always overwrites" edge is **unreachable** — a missing/invalid `device_id` yields `StateIdentityCorrupt`, not `StateOnline`. Good. - The new no-force branch is **reachable** (not dead code): `EnsureRootNotInitialized` returns nil when a registry already exists (`registry.go:277-297`), so the copy-`root.toml`-to-a-fresh-dir / join-existing-root workflow flows through to `Init` and exercises the new path. ### Confirmed issue — silent, mode-resetting clobber on a same-host re-register (medium) Pre-PR, *every* no-force re-init against an existing matching-`root_id` `root.toml` was refused with `INIT_ALREADY_INITIALIZED`. Post-PR, **any** differing `device_id` overwrites silently with exit 0. Concretely, a mistyped `--device-id` on the same machine (e.g. `laptop` → `laptpo`) now: 1. Silently overwrites the local `root.toml` (reproduced with a throwaway `config` test, since removed: differing device_id, `force=false`, `Init` returns nil and the file is rewritten). 2. **Resets `mode`**: `register.go` inherits `[remote]` from the existing `root.toml` via `loadRemoteDefaults`, but `mode` is **not** inherited — it falls back to the flag default `bidirectional` (`register.go:46`). A new/typo device that omits `--mode` silently downgrades a shared `upload-only` root to `bidirectional`. Verified: `mode "upload-only" -> "bidirectional"` with no warning. 3. Registers the typo'd id as a **phantom device** in the shared remote registry — `deviceregistry.Register(f.deviceID)` runs at `register.go:149`, *before* `Init`. This phantom write already happened pre-PR too, but pre-PR the command then exited non-zero (`INIT_ALREADY_INITIALIZED`), giving the user a signal. Post-PR the command exits 0, so **the only signal that a re-register went wrong is now gone**. Net: the safety net that caught *all* accidental re-inits now catches only the exact-same-`device_id` case, and the new path silently rewrites `mode`. (Note: `root.toml`'s `created_at` is also re-stamped on overwrite, but that field is write-only — never read for any decision, never surfaced in output, and stamped with each device's local `time.Now()` on every write since the schema's inception, so it has never represented a root-level creation time. Re-stamping it is harmless; not a finding.) ### Gap analysis (what CI/tests didn't cover) - `TestInit_newDeviceSameRootNoForce` only asserts the happy path (new device succeeds, `device_id` updated). It does **not** assert what happens to `mode` on overwrite, so the silent mode-reset is untested. - No test covers the asymmetry between `[remote]` inheritance and `mode` non-inheritance in the register command. - No test covers the typo-on-same-host case that the relaxation newly admits. ### Untested assumption / open question for dev - Is silently picking `bidirectional` for a device that omits `--mode` against a shared non-bidirectional root **intended**? If so, a one-line note in the register help / brief would close the gap. If not, `mode` should be inherited from the existing `root.toml` symmetrically with `[remote]`. ### Recommendation Send back to dev to either (a) inherit `mode` alongside `[remote]` on register, or (b) consciously document the new silent-overwrite semantics (mode reset on differing `device_id` without `--force`) in the PR body and register help, and add a test pinning the chosen behavior. ### Routing note (fjx) `fjx qa` exposes no hand-off/assign verb (only `next`, `ledger`, `checks`, `release`, `prompt`); `fjx issue` is `list`/`view`/`label` only — there is no `fjx qa done` mirroring `fjx dev done`. I'm therefore signalling the verdict via this ledger + the existing `agent/review` label rather than reassigning the issue myself. If QA is expected to reassign to `agent-dev` on a concerns verdict, that capability is missing at the CLI.
erik commented 2026-06-26 22:27:17 +00:00
Author
Owner
Copy link

@agent-pm after discussing with @agent-qa kick this back to @agent-dev to make sure that mode gets inherited from the root.toml as well - my example tricked him into thinking it was only the --remote-* fields. We can ignore the concerns about created_at and the typo-on-same-host

@agent-pm after discussing with @agent-qa kick this back to @agent-dev to make sure that `mode` gets inherited from the root.toml as well - my example tricked him into thinking it was only the --remote-* fields. We can ignore the concerns about created_at and the typo-on-same-host
agent-pm commented 2026-06-26 22:31:52 +00:00
Member
Copy link

Brief — Dev (rework)

Phase: simple. Run fjx dev prompt simple for the cycle protocol.

PR: #17. Resume the same worktree / <type>/<slug> branch / PR — this is rework on the existing diff, not a new branch or PR.

QA Findings

QA reviewed PR #17 (commit 45d9860) and returned concerns. After discussion, $FJX_OWNER has scoped this rework to a single fix — ignore the rest of the QA ledger's items.

Fix: inherit mode from an existing .dropbear/root.toml, symmetrically with the [remote] fields.

register already inherits the [remote] defaults from an existing root.toml via loadRemoteDefaults, but mode is not inherited — it falls back to the flag default bidirectional (register.go:46). The original issue example only mentioned the --remote-* fields, which is what made mode get missed; the intent is for a shared root.toml to seed all of its reusable defaults, mode included. As-is, a device that omits --mode against a shared upload-only root silently downgrades it to bidirectional.

Make mode inherit from the on-disk root.toml when --mode is not explicitly set on the command line, the same way [remote] defaults already do. Add a test pinning the behavior (the existing TestInit_newDeviceSameRootNoForce / register tests don't cover mode on the inheritance path).

Explicitly out of scope (do NOT address)

Per $FJX_OWNER, ignore these QA items entirely:

  • The created_at re-stamp on overwrite (write-only field, harmless).
  • The typo-on-same-host silent-overwrite concern. The relaxed config.Init no-force guard stays as-is.

Stay strictly within the mode-inheritance fix. When done, re-run just validate (skip only if no code changed), then fjx dev done 14.

<!-- pm:brief:dev --> # Brief — Dev (rework) **Phase:** simple. Run `fjx dev prompt simple` for the cycle protocol. PR: #17. **Resume the same worktree / `<type>/<slug>` branch / PR** — this is rework on the existing diff, not a new branch or PR. ## QA Findings QA reviewed PR #17 (commit 45d9860) and returned **concerns**. After discussion, `$FJX_OWNER` has scoped this rework to a **single** fix — ignore the rest of the QA ledger's items. **Fix: inherit `mode` from an existing `.dropbear/root.toml`, symmetrically with the `[remote]` fields.** `register` already inherits the `[remote]` defaults from an existing `root.toml` via `loadRemoteDefaults`, but `mode` is *not* inherited — it falls back to the flag default `bidirectional` (`register.go:46`). The original issue example only mentioned the `--remote-*` fields, which is what made `mode` get missed; the intent is for a shared `root.toml` to seed **all** of its reusable defaults, `mode` included. As-is, a device that omits `--mode` against a shared `upload-only` root silently downgrades it to `bidirectional`. Make `mode` inherit from the on-disk `root.toml` when `--mode` is not explicitly set on the command line, the same way `[remote]` defaults already do. Add a test pinning the behavior (the existing `TestInit_newDeviceSameRootNoForce` / register tests don't cover `mode` on the inheritance path). ### Explicitly out of scope (do NOT address) Per `$FJX_OWNER`, ignore these QA items entirely: - The `created_at` re-stamp on overwrite (write-only field, harmless). - The typo-on-same-host silent-overwrite concern. The relaxed `config.Init` no-force guard stays as-is. Stay strictly within the `mode`-inheritance fix. When done, re-run `just validate` (skip only if no code changed), then `fjx dev done 14`.
agent-dev removed their assignment 2026-06-26 22:39:18 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
add-daemon-lifecycle-control
v0.5.0
v0.4.3
v0.4.2
v0.4.1
v0.4.0
v0.3.0
v0.1.2
v0.1.1
v0.1.0
Labels
Clear labels
  
agent
blocked
 Needs human intervention

  
agent
new
 Waiting triage by PM

  
agent
review
 Awaiting next phase

  
agent
working
 Agent is actively implementing

  
complexity
high
 High complexity - use more capable LLM

  
complexity
low
 Low complexity - use simpler LLM

  
priority
high
 High priority

  
priority
low
 Low priority

  
priority
medium
 Medium priority

  
risk
high
 High risk — strict QA, no auto-release

  
risk
low
 Low risk — standard flow

  
risk
medium
 Medium risk — standard QA

  
type
bug
 Bug fix

  
type
chore
 Maintenance, deps, tooling

  
type
feature
 New feature — spec required

  
type
security
 Security change — spec + strict QA required

No labels
agent
blocked
agent
new
agent
review
agent
working
complexity
high
complexity
low
priority
high
priority
low
priority
medium
risk
high
risk
low
risk
medium
type
bug
type
chore
type
feature
type
security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
agent-pm
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
tfks/dropbear#14
No description provided.