connections: ?move=true moves the matched NetBox entry with its credentials

[jseifeddine]

Based on current main on its own: one commit, nothing from any other open PR. Part 1/3 of #253.

What it does

PUT /api/addressbook/folders/{scope}/{folder}/entries/{entry}?move=true is for sync sources such as the NetBox webhook, where the folder (one per tenant) or the name is computed from the device and can change. Entries now carry a stored external_id (the webhook body sends netbox:device:{{ data.id }}), and the flag matches the device on that, never on its name.

Matching rule

1. folder/entry exists with the same external_id, or with none (an entry made by hand before syncing began): updated in place.
2. Otherwise the scope is scanned for the entry carrying that external_id, and it is moved to folder/entry by the existing server-side move (move_stored_entry, the copy-then-delete path behind POST .../entries/{entry}/move from v1.10.3), so its password, key, jump host credentials and $variable references come along; the update is then merged on top. Response: {"ok": true, "moved_from": {"folder": "...", "name": "..."}}.
3. No entry carries it: created.

Same-named entries in other folders are never touched. 400 without external_id; 409 if folder/entry belongs to a different id, or the id is stored on more than one entry (nothing is changed either way); 502 with "copied": true if the move copied the entry but could not remove the original. Without the flag the PUT is the plain name upsert it always was. The scan only runs when the entry is not already where the request says (first sight of a device, a tenant change, a rename), so a routine update is one read and one write as before.

Two smaller things in the same merge: external_id is kept when the request omits it (the edit form never sends it, and it must not cut a synced entry loose from its source), and a failed read of the existing entry is now a 502 instead of a blind write, so a payload without secrets can no longer replace an entry that has them.

Review changes

• Deleted every same-named entry in the scope. Gone. Nothing is matched or deleted by name across folders; the only delete is the source half of moving the one entry that carries the matching external_id. Tests: move_upsert_leaves_a_same_named_entry_in_another_folder_alone, move_upsert_refuses_a_name_held_by_another_device, move_upsert_needs_an_external_id, plain_upsert_never_moves.
• Old entry deleted after the new one was written from a payload with no secrets (Bug - When a entry is moved to a new folder, the password entry does not move #232). The matched entry is moved first through move_stored_entry, the same code as the move endpoint (full entry copied with put_entry_if_absent, original removed only after), and the payload is merged onto the moved entry afterwards, so every secret the payload lacks is kept. Tests: move_upsert_moves_the_matched_entry_with_its_credentials, move_upsert_follows_a_rename_within_the_folder, upsert_without_secrets_keeps_the_stored_ones, move_upsert_reports_a_move_it_could_not_finish.
• Match on a stored NetBox id. There was no such field, so this adds external_id to AddressBookEntry (src/vault.rs, also returned in EntryInfo), populated by the webhook body in docs/netbox.md; relocate_by_external_id in src/api.rs does the matching, upsert_entry the sequence.
• Rebased onto main on its own. Done; the branch is origin/main plus this commit.

Tests

11 unit tests in src/api.rs run the locate / move / merge sequence against an in-memory EntryStore (the handler's Vault access sits behind that trait, like the existing FolderSource): the eight named above plus move_upsert_adopts_a_hand_made_entry_of_that_name, move_upsert_refuses_an_ambiguous_external_id and entry_put_query_parses_the_move_flag.

cargo fmt clean, cargo clippy --all-targets no warnings. cargo test: 342 passed, 2 failed, the two vault::tests::test_build_client_mtls_* tests, which fail identically on main in the same container (UnsupportedCertVersion from the locally generated test certificate) and are unrelated to this change.

Docs

docs/api.md: the external_id field and the ?move=true rules and responses under PUT .../entries/:entry. docs/netbox.md: the create/update webhook is now PUT ...?move=true with external_id in the body, with the per-tenant folder example.

[davekempe]

Thanks Jad. Can't take this one as is, two problems:

• It deletes every same-named entry in the scope. Names aren't unique across folders (two tenants can both have a fw01), so one webhook for tenant A's fw01 would delete tenant B's, credentials and all.
• When a device does move, the old entry is deleted after the new one is written from the webhook, which has no secrets, so any password or key someone added by hand is lost. That's the same bug as Bug - When a entry is moved to a new folder, the password entry does not move #232.

Could it match the device by a stored NetBox ID rather than the name, and move the existing entry with the server-side move from v1.10.3 (POST .../entries/{entry}/move) so its credentials come along, then apply the update?

Also, since the PRs are stacked, could you rebase this onto main on its own so it can be taken separately?

[jseifeddine]

Thanks Dave, both points were fair and this version does it the way you suggested. Entries now carry a stored external_id (the webhook sends netbox:device:) and ?move=true matches on that only, so tenant B's fw01 is never touched by tenant A's webhook and nothing is ever deleted by name. When the id is found in another folder or under another name, the entry goes through the same copy-then-delete move as the v1.10.3 move endpoint, so its password, key, jump host credentials and $variable references come along, and only then is the webhook payload merged on top, keeping every secret it lacks. A name already held by a different device, or an id stored on two entries, is a 409 with nothing changed. It is rebased onto main on its own now; the matching rules are under PUT .../entries/:entry in docs/api.md and the updated webhook template is in docs/netbox.md.

The CI audit failure on this branch is the new quinn-proto advisories, not this change; #283 bumps the lockfile and the run goes green once that is in.