Repository navigation
Secrets: write-only entry, split identities — operator verbs, design doc, CI guard - #5790
Conversation
…or verbs, doc, CI guard Policy `secrets-write-only-entry`: nobody touches Key Vault by hand. Secrets are entered through the write-only GUI of the app that owns them and written by ONE governed writer identity that can list, set, delete and recover but never read a value; the pods read through a get-only reader. - hosting-kv-set: stamps mw-fp (SHA-256 prefix, withheld for low-entropy values), mw-set-by, mw-set-at and mw-source tags at write time; --generate mints a value in the Job that nobody sees; confirms the write by version through list-versions and never reads a value back. - hosting-kv-status (new): status from vault METADATA only (existence, enabled, dates, soft-delete, write-time tags). - hosting-kv-state (new): enable/disable/delete/recover/purge; purge only when already soft-deleted. - manifests/hosting-operator/secret-writer.yaml: the writer's ServiceAccount and narrow ClusterRole. - Doc/Architecture/SecretsWriteOnlyEntry + PolicyNotProse row; manual vault commands removed from core docs and the deployment skills in favour of the Deployments page's secret dialog. - .github/scripts/check-manual-keyvault.py: CI refuses a new hand-run vault command outside the governed verbs (allow file) or a marked kv-break-glass procedure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical issues affect the CI guard, workflow tests, RBAC scope, and lifecycle safety.
Review effort: Lite
Findings: 5
Open (7)
What changed in this PR
Adds a proposed write-only Key Vault workflow with split writer/reader identities, operator verbs, documentation, RBAC, tests, and CI enforcement.
Changes:
- Adds metadata-only status and lifecycle operations with fingerprinted writes.
- Adds writer identity manifests, behavior tests, and workflow declarations.
- Updates deployment guidance and guards against manual Key Vault commands.
| File | Summary |
|---|---|
src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateAnnouncementKey.md |
Updates secret-entry guidance. |
src/MeshWeaver.Documentation/Data/Architecture/SecretsWriteOnlyEntry.md |
Documents the write-only design and migration. |
src/MeshWeaver.Documentation/Data/Architecture/PolicyNotProse.md |
Registers the proposed policy. |
src/MeshWeaver.Documentation/Data/Architecture/OperatorCredentialsByName.md |
Documents migration limitations. |
src/MeshWeaver.Documentation/Data/Architecture/OnboardingNewEnvironment.md |
Replaces manual vault procedures. |
src/MeshWeaver.Documentation/Data/Architecture/MemexCloudDeployment.md |
Updates secret rotation guidance. |
src/MeshWeaver.Documentation/Data/Architecture/InstanceIdentityAndSetup.md |
Removes manual vault references. |
src/MeshWeaver.Documentation/Data/Architecture/DeploymentOptions.md |
Updates secret setup instructions. |
src/MeshWeaver.Documentation/Data/Architecture/DependabotSecretStore.md |
Removes manual vault handling. |
src/MeshWeaver.Documentation/Data/Architecture.md |
Links the new architecture page. |
deploy/aks/operator/test/stubs/kv-status/az |
Adds the status-command test stub. |
deploy/aks/operator/test/stubs/kv-set/az |
Updates write-command test behavior. |
deploy/aks/operator/test/run-tests.sh |
Adds operator behavior tests. |
deploy/aks/operator/bin/hosting-kv-status |
Implements metadata-only status reporting. |
deploy/aks/operator/bin/hosting-kv-state |
Implements lifecycle operations. |
deploy/aks/operator/bin/hosting-kv-set |
Implements write-only secret setting. |
deploy/aks/operator/bin/_common.sh |
Provides fingerprint and actor helpers. |
deploy/aks/manifests/hosting-operator/secret-writer.yaml |
Adds the writer identity and RBAC. |
deploy/aks/manifests/hosting-operator/README.md |
Documents the writer identity. |
.github/workflows/hosting-operator.yml |
Declares commands and validates stubs. |
.github/workflows/dotnet-test.yml |
Integrates the manual-command guard. |
.github/scripts/check-manual-keyvault.py |
Scans for prohibited vault commands. |
.github/manual-keyvault.allow |
Defines command exceptions. |
.claude/skills/new-deployment/SKILL.md |
Updates deployment secret instructions. |
.claude/skills/deployment/SKILL.md |
Updates operational guidance. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # matches a file holding a vault command is stale and reds the gate. A doc or runbook NEVER goes | ||
| # here — a documented break-glass procedure carries the `kv-break-glass` marker instead. | ||
|
|
||
| deploy/aks/operator/bin/* — the GOVERNED implementation: the operator verbs an InstanceAction runs (hosting-kv-set/-status/-state as the writer identity; the provisioning verbs still on hosting-operator are the migration's step 3) |
There was a problem hiding this comment.
Fixed in 30840bb. The allow file no longer has a glob. It names each governed file with its reason, and the value readers (kv-ensure, kv-copy, kv-rotate, registry-register, signin-app, image-mirror, pull-secret, hosting-deploy, _common.sh's pg_password) are marked TRANSITIONAL. The migration's step 2 removes each read, and its line goes with it. The hand-command fallback strings in kv-ensure, kv-copy, signin-app and registry-register now point at the write-only GUI instead of an az keyvault secret set, and the behaviour tests assert the new text.
| PATTERN = re.compile( | ||
| r"az\s+keyvault\s+(?:secret\s+(?:set-attributes|set|show|download|delete|purge|recover|backup|restore)\b" | ||
| r"|set-policy\b[^\n]*--secret-permissions)") |
There was a problem hiding this comment.
Fixed in 30840bb. logical_lines() joins lines that end in a trailing backslash before matching. Two self-test cases cover a command split after secret and one split after keyvault, and both fire.
| window = lines[max(0, i - MARKER_WINDOW): i + 1] | ||
| if any(MARKER in w for w in window): | ||
| continue |
There was a problem hiding this comment.
Fixed in 30840bb. The marker now exempts a command only in the form kv-break-glass: <reason> with at least 20 characters of reason. A bare marker and a too-short reason each have a self-test case that fires.
| run: | | ||
| set -euo pipefail | ||
| for f in deploy/aks/operator/test/stubs/pull-secret/az deploy/aks/operator/test/stubs/pull-secret/kubectl deploy/aks/operator/test/stubs/pv-resize/kubectl deploy/aks/operator/test/stubs/inline-env/kubectl deploy/aks/operator/test/stubs/kv-rotate/az deploy/aks/operator/test/stubs/kv-ensure/az deploy/aks/operator/test/stubs/kv-copy/az deploy/aks/operator/test/stubs/registry-register/az deploy/aks/operator/test/stubs/signin-app/az deploy/aks/operator/test/stubs/image-mirror/az deploy/aks/operator/test/stubs/image-mirror/crane deploy/aks/operator/test/stubs/deploy/az deploy/aks/operator/test/stubs/kv-set/az deploy/aks/operator/test/stubs/kv-set/kubectl deploy/aks/operator/test/stubs/db-release/kubectl deploy/aks/operator/test/stubs/db-release/helm deploy/aks/operator/test/stubs/pg/az deploy/aks/operator/test/stubs/pg/pg_dump deploy/aks/operator/test/stubs/pg/psql deploy/aks/operator/test/stubs/pg/pg_restore deploy/aks/operator/test/stubs/aks-upgrade/az deploy/aks/operator/test/stubs/aks-upgrade/kubectl deploy/aks/operator/test/stubs/aks-upgrade/curl; do | ||
| for f in deploy/aks/operator/test/stubs/pull-secret/az deploy/aks/operator/test/stubs/pull-secret/kubectl deploy/aks/operator/test/stubs/pv-resize/kubectl deploy/aks/operator/test/stubs/inline-env/kubectl deploy/aks/operator/test/stubs/kv-rotate/az deploy/aks/operator/test/stubs/kv-ensure/az deploy/aks/operator/test/stubs/kv-copy/az deploy/aks/operator/test/stubs/registry-register/az deploy/aks/operator/test/stubs/signin-app/az deploy/aks/operator/test/stubs/image-mirror/az deploy/aks/operator/test/stubs/image-mirror/crane deploy/aks/operator/test/stubs/deploy/az deploy/aks/operator/test/stubs/kv-set/az deploy/aks/operator/test/stubs/kv-set/kubectl deploy/aks/operator/test/stubs/kv-status/azdeploy/aks/operator/test/stubs/db-release/kubectl deploy/aks/operator/test/stubs/db-release/helm deploy/aks/operator/test/stubs/pg/az deploy/aks/operator/test/stubs/pg/pg_dump deploy/aks/operator/test/stubs/pg/psql deploy/aks/operator/test/stubs/pg/pg_restore deploy/aks/operator/test/stubs/aks-upgrade/az deploy/aks/operator/test/stubs/aks-upgrade/kubectl deploy/aks/operator/test/stubs/aks-upgrade/curl; do |
There was a problem hiding this comment.
Fixed in 30840bb: the separator is restored. The bin/ tracked-file floor is also raised to 34 for the two new verbs.
| - apiGroups: [""] | ||
| resources: ["secrets"] | ||
| verbs: ["get"] |
There was a problem hiding this comment.
Fixed in 30840bb. The Secret rule is gone. hosting-kv-set --wait now decides from secretproviderclasspodstatuses VERSIONS, which are metadata: every pod mount must report the version the write just created. The writer's ClusterRole holds no Secret read anywhere. The kubectl stub refuses get secret, so a regression is red. operator-rbac.yaml gains a read-only rule for the pod statuses, for as long as a secret write can still run as hosting-operator.
| exactly the reader this design removes. `MeshWeaver.Mesh.SecretFingerprint.Of` and the operator's | ||
| `hosting::fingerprint` compute the same string. |
There was a problem hiding this comment.
Fixed in 30840bb. The page now calls the operator's hosting::fingerprint the reference implementation, and states that the portal-side SecretFingerprint.Of is OWED by the companion core change (the write-only secret control), which must match it byte for byte. The exact rule has been handed to that change's author.
| A stale allow entry fails too, so the list only shrinks. Core runs the guard on itself. Every node repo | ||
| gets it through the shared `node-repo-validate` lane, and Systemorph/Memex runs it in its own build. |
There was a problem hiding this comment.
Fixed in 30840bb. The page now says core runs the guard on itself, and that node repos and Memex do not run it yet: wiring node-repo-validate waits for the Plugins cleanup, because doing it first would red every Plugins PR.
…wed allow list, continuation + reasoned break-glass in the guard, no awk in the image - hosting-kv-set --wait reads SecretProviderClassPodStatus versions (metadata); the writer's ClusterRole drops its Secret get; operator-rbac gains the read-only pod-status grant. - hosting::fingerprint uses pure bash arithmetic (Azure Linux 3 carries no awk). - Hand-command fallbacks in kv-ensure/kv-copy/signin-app/registry-register now point at the write-only GUI. - check-manual-keyvault: joins continued lines; a marker exempts only with a >=20-char reason; allow list names each governed file, value readers marked TRANSITIONAL. - Design doc: owed components marked owed, lane wiring stated as deferred, migration plan incl. the RBAC-mode fleet vault (step 4). - hosting-operator.yml: stub list separator, bin floor 34. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… GUI Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What
Policy
secrets-write-only-entry(registered as proposed in PolicyNotProse, with the owed items listed): nobody accesses Key Vault by hand.get.Design page:
Doc/Architecture/SecretsWriteOnlyEntry.Operator (core half of the governed write path)
hosting-kv-setmw-fp(sha256:<12 hex>, orwithheld:low-entropybelow 128 estimated bits),mw-set-by,mw-set-at,mw-source;--generatemints a value in the Job that nobody ever sees;--set-byrecords the requester;list-versions(metadata).hosting-kv-status(new): status fromlist/list-deletedmetadata only.hosting-kv-state(new): enable, disable, delete, recover and purge. Purge is refused unless the object is already soft-deleted. It passes no tags, becauseset-attributeswould wipe the fingerprint.azplays the writer and refusessecret show. 648/648 locally, and shellcheck is clean.hosting-operator.yml; the plan half is the Plugins PR.manifests/hosting-operator/secret-writer.yaml: the writer's ServiceAccount (federated tosecret-writer) and its narrow ClusterRole.getused for the hash wait, is stated there.CI guard
.github/scripts/check-manual-keyvault.py+.github/manual-keyvault.allow: refuses any tracked doc, runbook, skill or script that holds a Key Vault secret data-plane command. Exceptions are the governed verbs (allow file) and a procedure markedkv-break-glass. A stale allow entry is red.dotnet-test.ymlguard block.node-repo-validate.yml: MeshWeaver.Plugins still carries such commands, so wiring it now would red every Plugins PR. That wiring follows once the Plugins cleanup PR merges.Docs
Manual vault commands are removed from: DependabotSecretStore, DeploymentOptions, InstanceIdentityAndSetup, MemexCloudDeployment, OnboardingNewEnvironment, OperatorCredentialsByName, SelfUpdateAnnouncementKey, and the
/deploymentand/new-deploymentskills. Each now points at the Deployments page's Set Key Vault secrets… dialog.Pairs-with: none — no public type or member removed.
Mirror-sync: none — no i18n catalog key added or changed.
Counterparts: the Systemorph/Memex PR (writer identity in
infra/estate.bicep,docs/control-instance.md, runbooks) and the MeshWeaver.Plugins PR (status/lifecycle actions, writer ServiceAccount selection, GUI). Both follow this one.🤖 Generated with Claude Code