Repository navigation
hosting-kv-copy: materialise a fleet-shared vault object under an instance's prefix (Plugins#1723) - #4103
Conversation
…tance's prefix (MeshWeaver.Plugins#1723) The fleet GitHub App's private key existed only under the control instance's prefix (memexsystemorph-GitHub-App-PrivateKey); the build instance maps it as build-GitHub-App-PrivateKey, and the copy was a hand step (az show | az set --file /dev/stdin). The copy is needed because hosting-kv-purge deletes by PREFIX on teardown — mapping across prefixes would let the first teardown take the shared PEM. hosting-kv-copy --vault V --copy <target>=<source> [...]: an absent target is copied from its source through a mode-600 --file (never argv), read back by id; an existing target is KEPT — never rewritten — and compared with its source by sha256, a difference reported as ::hosting:: kv_copy_drift (a fact, not a failure). No value is ever printed. Every pair is validated before the vault is touched; every refusal names the exact hand command. The plan half (keyVaultSecrets.secrets[].copyFrom → one step after the vault step) is the paired MeshWeaver.Plugins PR. hosting-operator.yml: plan list + tracked-file floor (24) + stub list. Tests: 28 behaviour cases against a recorded-argv az stub (test/stubs/kv-copy/az). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate correctness and safety issues remain in the copy command.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds hosting-kv-copy to materialize fleet-shared Key Vault secrets under instance-specific prefixes while preserving existing targets.
Changes:
- Adds copy, keep/drift, hashing, and dry-run behavior.
- Adds tests and an Azure CLI stub.
- Updates lifecycle documentation and CI tracking.
File summaries
| File | Summary |
|---|---|
deploy/aks/operator/test/stubs/kv-copy/az |
Azure CLI test stub; moderate concern that whitespace splitting prevents byte-preserving and trailing-newline coverage. |
deploy/aks/operator/test/run-tests.sh |
Adds command behavior and safety tests. |
deploy/aks/operator/bin/hosting-kv-copy |
Implements copying; findings include critical source-read, byte-preservation, and write-error handling issues; moderate argument and target-read handling issues; and a nit in the refusal message. |
deploy/aks/INSTANCE-LIFECYCLE.md |
Documents the lifecycle behavior. |
.github/workflows/hosting-operator.yml |
Adds CI integration; moderate issue that the tracked-file floor and diagnostic should be 25 rather than 24. |
Review details
Suppressed comments (1)
deploy/aks/operator/test/stubs/kv-copy/az:16
- Because this expands
HOSTING_KVC_VALUESunquoted, the stub splits on whitespace and cannot represent the multiline/trailing-newline PEM this command is meant to copy. The new “byte-for-byte” test therefore only exercises a one-line token and will not catch the-o tsvtruncation; use a representation that preserves arbitrary secret bytes and add a trailing-newline case.
for pair in ${HOSTING_KVC_VALUES:-}; do
if [ "${pair%%=*}" = "$1" ]; then printf '%s' "${pair#*=}"; return 0; fi
- Files reviewed: 4/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Corrected the tracked-file floor in 🤖 Generated with Claude Code |
|
Correction to the comment above: this PR keeps the floor at 24, not 25 — the force-push was declined because the PR had already entered the merge queue ( The consequence is one file of slack in the guard (real count after this merges: 25, floor: 24) — exactly the slack 🤖 Generated with Claude Code |
#4100 (hosting-kv-ensure --db-connection) landed first and touched the same three places. All three conflicts were "both sides added": the INSTANCE-LIFECYCLE guarantee list keeps main's extended kv-ensure bullet AND the kv-copy one; the behaviour suite keeps both blocks (346 passed, 0 failed = 319 + main's 27); the workflow's tracked-stub list is the union of both stubs. The tracked-file floor is also CORRECTED rather than carried: main declared 23 while already tracking 24 (the comment said "20 commands" — stale, so one lost script would have passed). With hosting-kv-copy the real count is 25, and the floor and its comment now say 25 (22 commands + run.sh + _common.sh + _audit.jq). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merge resolved by content; floor settled at 25 — measured, not assumed.
The floor, and how it was counted. Guards run on the pushed head: The follow-up is still needed and is now precisely scoped. Each of #4103/#4105/#4109/#4110 sets the floor true for its own merge, so as the siblings land the real count rises 25 → 26 → 27 → 28 while the floor stays 25 — up to 3 files of slack. After all four have merged, a one-line PR sets 🤖 Generated with Claude Code |
…d at 26 Conflicts resolved by content, keeping both sides: - .github/workflows/hosting-operator.yml: tracked-stub list carries both kv-copy/az (this PR) and signin-app/az (main, Plugins#1719). - deploy/aks/INSTANCE-LIFECYCLE.md: both the hosting-kv-copy and the hosting-signin-app bullets. - deploy/aks/operator/test/run-tests.sh: both the kv-copy and the signin-app behaviour blocks (380 passed, 0 failed on the merged tree). The tracked-file floor in hosting-operator.yml is the TRUE count on the merged tree, not either side's: `git ls-files deploy/aks/operator/bin | wc -l` on this tree = 26 (23 commands + run.sh + _common.sh + _audit.jq). main is at 25 (22 commands); this PR adds hosting-kv-copy, so 26. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRLdhAiX5DjdZC2WUfPg9E
…or measured at 26 Conflicts resolved by content, keeping both sides: - .github/workflows/hosting-operator.yml: tracked-stub list carries both registry-register/az (this PR) and signin-app/az (main, Plugins#1719). - deploy/aks/INSTANCE-LIFECYCLE.md: both the hosting-registry-register and the hosting-signin-app bullets. - deploy/aks/operator/test/run-tests.sh: both the registry-register and the signin-app behaviour blocks (390 passed, 0 failed on the merged tree). The tracked-file floor in hosting-operator.yml is the TRUE count on the merged tree, not either side's: `git ls-files deploy/aks/operator/bin | wc -l` on this tree = 26 (23 commands + run.sh + _common.sh + _audit.jq). main is at 25 (22 commands); this PR adds hosting-registry-register, so 26. Once #4103 (hosting-kv-copy) also lands, main tracks 27 and the floor stays a valid lower bound; the next command added re-measures it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRLdhAiX5DjdZC2WUfPg9E
…register — floor measured at 27 #4103 (hosting-kv-copy) merged first and touched the same three files, so the same conflicts came back; resolved by content, keeping every side: - .github/workflows/hosting-operator.yml: tracked-stub list carries kv-copy/az (main), registry-register/az (this PR) and signin-app/az. - deploy/aks/INSTANCE-LIFECYCLE.md: the hosting-registry-register, hosting-kv-copy and hosting-signin-app bullets. - deploy/aks/operator/test/run-tests.sh: the registry-register, kv-copy and signin-app behaviour blocks (418 passed, 0 failed on the merged tree). The tracked-file floor is re-measured on THIS merged tree: `git ls-files deploy/aks/operator/bin | wc -l` = 27 (24 commands + run.sh + _common.sh + _audit.jq). main is at 26 since #4103; this PR adds hosting-registry-register, so 27. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRLdhAiX5DjdZC2WUfPg9E
Operator half of MeshWeaver.Plugins#1723 (the plan half —
keyVaultSecrets.secrets[].copyFrom→ a "Copy fleet-shared secrets" step — is the paired Plugins PR;Pairs-with: none — this PR removes no public surface).The hand step it replaces
Memex
docs/new-deployment.mdhas no step for it — it was done ad hoc forbuildon 2026-09-12 (build.jsonnotes):The Memex PR at the end of this series adds it to the runbook as "done by Provision since Plugins#1723".
What changes
New command
hosting-kv-copy --vault <v> --copy <target>=<source> […]:--file(never argv), read back by id;::hosting:: kv_copy_created=n;kv_copy_drift=n— a fact for the operator, not a failure;Why a copy at all:
hosting-kv-purgedeletes by PREFIX on teardown, so a cross-prefix mapping would let the first teardown take the fleet's shared PEM with it. The copy makes the object the instance's own, purged with it.Rights the operator identity needs
Key Vault secret get on the source object and set on the target — both under the existing Key Vault Secrets Officer grant on
Systemorph. Nothing new.Gates run
hosting-operator.yml:hosting-kv-copyadded to the plan-command list, tracked-file floor raised to 26 (measured on the tree merged with main 8aaf35f: 23 commands + run.sh + _common.sh + _audit.jq; main is at 25), stub added to the tracked-stub list.Ships how
Hop (C): published as
hosting-operator:<sha>on the merge tomain; reaches a Provision whenoperator.imageonDeployments/memexmoves. Merging is not shipping.🤖 Generated with Claude Code