Repository navigation
test(secrets): pin Secret and credential-source checks that no test detected - #1401
Merged
Merged
Conversation
…etected Mutation sweep of Secret and credential-source paths (test-gaps-4). Each new assertion kills a mutant that survived every relevant non-PG and PG file: - secret-occ: core Secret value checks on create and update (NUL, lone surrogate, byte limit) before the driver sees the value; a duplicate-name commit failure removes the backend copy; an update refuses a Secret owned by another Secret Driver; Harness Secret backend identity drift fails deploy. - credential-source-occ: a deletion that wins against a registration removes the late gateway copy and commits no audit; only a ready source updates; core read authorization and per-source list filtering; a failing gateway status read reports a fixed reason. - kubernetes-secret-driver: foreign Namespace annotation and a record owned by another Secret Driver are refused. - postgres-secret-state: Namespace-scoped Secret lookups and binding checks for a foreign Secret ID; an Agent's Harness binding retains its Secret. - secret-api: credential source create, update and delete each commit one value-free audit row.
Past-the-fence clock step matches the registration fence (71 s), comments sit over the assertions they describe, the race test checks committed audit once, and the deleting-source copy check is labelled as a precondition.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P3
Contributor
|
Codex review: needs maintainer review before merge. Reviewed October 5, 2026, 1:16 PM ET / 17:16 UTC. ClawSweeper reviewWhat this changesAdds regression tests for Secret validation, ownership, cleanup, tenant-scoped storage, credential-source authorization, and value-free HTTP audit records. Merge readiness✅ Ready for maintainer review The added checks protect distinct gaps in existing coverage; no blocking patch defect was found. Repository policy keeps this PR open for maintainer triage. Likely related people: freeqaz, mrunalp, and kevinlin-openai are medium-confidence routing candidates based on prior main-branch work. Priority: P3 Review scores
Verification
How this fits togetherOpenClaw Control Plane authorizes Secret and credential-source requests, stores their metadata, and delegates credential custody to selected Drivers. These resources supply credentials to Agent runtimes while keeping values out of public responses and audit records. flowchart TB
A[Authenticated requests] --> B[Control Plane authorization]
B --> C[Secret and source lifecycle]
C --> D[Metadata and audit storage]
C --> E[Secret Driver]
E --> F[Credential Gateway]
F --> G[Agent runtime credentials]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain focused regression coverage at the existing controller, Driver, HTTP, and storage boundaries without adding production seams. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR strengthens existing tests rather than fixing a reported runtime failure; mutant failures and repeated successful runs are author-reported. Is this the best way to solve the issue? Yes: the assertions exercise existing behavior owners and extend shared storage coverage without adding test-only production interfaces. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 029c3bb67505. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A mutation sweep of the Secret and credential-source paths (105 hand-written mutants over OCC
core, PostgreSQL and in-memory State, the HTTP handlers, the Kubernetes Secret Driver, the
OpenShell Credential Gateway, the worker's runtime materialization and the repository credential
broker) found checks that no test detects. Each mutant ran against every non-PG file that reaches
the mutated file (95) and the 22
postgres-platformfiles pluspostgres-worker-agent-revision,each PG file on a fresh clone of its prepared database. 58 of 105 were killed.
Change
Test-only. Each new assertion kills one or more surviving mutants:
readycredential source can be updatedfindSecret/lockSecretand binding availability for a foreign Secret IDhasReferences)The PG assertions extend existing tests, so
scripts/ci/test-suitesis unchanged.Kills by area, before -> after: Secret core 8 -> 12/14, HTTP 4 -> 7/9, bindings and Harness
auth 10 -> 11/16, credential sources 16 -> 21/21, State 8 -> 11/12, Kubernetes Secret Driver
6 -> 8/9. Total 58 -> 76 of 105. The OpenShell gateway, worker projection and broker
survivors are triaged separately.
Verification
secret-api; postgres-secret-state 20/20 on fresh database clones.
prettier --checkandeslint --max-warnings 0on the touched files.git merge-treeagainst this head showsthe same conflicted files and hunks as against main.
Risks
Test-only. The credential-source race test drives the deletion from inside the test gateway's
registerSource, so it is deterministic. The HTTP test uses an injected clock to pass theregistration fence. The recording gateway in secret-api duplicates part of the conformance
gateway; moving both into a shared helper is left for later, because #913 and #851 edit that
fixture.