test(secrets): concierge no-self-secret-ops regression tests (#2605) #2624

Merged
devops-engineer merged 1 commits from fix/concierge-no-self-secret-ops-test into main 2026-06-12 04:25:19 +00:00
Member

Closes molecule-core#2615.

Adds regression tests for the concierge no-self-secret-ops / safe-approval guard introduced in molecule-core#2605 and enforced by the core#2573 auto-restart skip.

The guard has two layers:

  1. Prompt guardrail in conciergeSystemPromptTmpl (platform_agent.go) — tells the concierge never to run secret ops against its own workspace.
  2. Code enforcement in autoRestartAllowed (secrets.go) — suppresses auto-restart when a secret write/delete targets the caller's own workspace or the org root (kind='platform').

These tests cover the code enforcement layer:

  • TestSecretsSet_NoAutoRestart_SelfWrite — caller == target workspace → no auto-restart.
  • TestSecretsSet_NoAutoRestart_PlatformRoot — target is the concierge (kind='platform') → no auto-restart.
  • Existing TestSecretsSet_AutoRestart already covers the safe path: a regular workspace write still triggers auto-restart.

Test plan

cd workspace-server
go test ./internal/handlers/ -run 'TestSecretsSet_(AutoRestart|NoAutoRestart)' -v

🤖 Generated with Claude Code


SOP checklist

  • Comprehensive testing performed: added two unit tests covering self-write and platform-root auto-restart suppression; existing TestSecretsSet_AutoRestart covers the safe path. Verified the new tests pass in CI.
  • Local-postgres E2E run: N/A — this change only adds unit tests to workspace-server/internal/handlers/secrets_test.go.
  • Staging-smoke verified or pending: N/A — no runtime/staging surface changed.
  • Root-cause not symptom: N/A — this is a regression-test addition for #2605.
  • Five-Axis review walked: correctness (self-write skip), security (platform-root skip), robustness (fail-closed on kind lookup failure already covered by autoRestartAllowed), readability, scope.
  • No backwards-compat shim / dead code added: yes.
  • Memory consulted: feedback_no_such_thing_as_flakes, reference_merge_gate_model_changed_2026_05_18.
Closes molecule-core#2615. Adds regression tests for the concierge no-self-secret-ops / safe-approval guard introduced in molecule-core#2605 and enforced by the core#2573 auto-restart skip. The guard has two layers: 1. Prompt guardrail in `conciergeSystemPromptTmpl` (`platform_agent.go`) — tells the concierge never to run secret ops against its own workspace. 2. Code enforcement in `autoRestartAllowed` (`secrets.go`) — suppresses auto-restart when a secret write/delete targets the caller's own workspace or the org root (`kind='platform'`). These tests cover the code enforcement layer: - `TestSecretsSet_NoAutoRestart_SelfWrite` — caller == target workspace → no auto-restart. - `TestSecretsSet_NoAutoRestart_PlatformRoot` — target is the concierge (`kind='platform'`) → no auto-restart. - Existing `TestSecretsSet_AutoRestart` already covers the safe path: a regular workspace write still triggers auto-restart. ### Test plan ```bash cd workspace-server go test ./internal/handlers/ -run 'TestSecretsSet_(AutoRestart|NoAutoRestart)' -v ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ### SOP checklist - Comprehensive testing performed: added two unit tests covering self-write and platform-root auto-restart suppression; existing `TestSecretsSet_AutoRestart` covers the safe path. Verified the new tests pass in CI. - Local-postgres E2E run: N/A — this change only adds unit tests to `workspace-server/internal/handlers/secrets_test.go`. - Staging-smoke verified or pending: N/A — no runtime/staging surface changed. - Root-cause not symptom: N/A — this is a regression-test addition for #2605. - Five-Axis review walked: correctness (self-write skip), security (platform-root skip), robustness (fail-closed on kind lookup failure already covered by `autoRestartAllowed`), readability, scope. - No backwards-compat shim / dead code added: yes. - Memory consulted: `feedback_no_such_thing_as_flakes`, `reference_merge_gate_model_changed_2026_05_18`.
agent-dev-a added 1 commit 2026-06-12 04:15:19 +00:00
test(secrets): regression tests for concierge no-self-secret-ops guard (#2605)
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 3s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 3s
E2E API Smoke Test / detect-changes (pull_request) Successful in 5s
E2E Chat / detect-changes (pull_request) Successful in 5s
CI / Python Lint & Test (pull_request) Successful in 7s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
E2E Chat / E2E Chat (pull_request) Successful in 2s
Harness Replays / detect-changes (pull_request) Successful in 6s
Harness Replays / Harness Replays (pull_request) Successful in 1s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 7s
reserved-path-review / reserved-path-review (pull_request_target) Successful in 8s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 15s
CI / Detect changes (pull_request) Successful in 16s
CI / Canvas (Next.js) (pull_request) Successful in 2s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 2s
CI / Canvas Deploy Status (pull_request) Successful in 1s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 24s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (stub) (pull_request) Successful in 33s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 35s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (real image + MiniMax LLM, advisory) (pull_request) Failing after 26s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m19s
sop-checklist / review-refire (pull_request_target) Has been skipped
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request_target) Successful in 3s
CI / Platform (Go) (pull_request) Successful in 2m49s
gate-check-v3 / gate-check (pull_request_target) Failing after 14s
CI / all-required (pull_request) Successful in 1s
qa-review / approved (pull_request_target) Approved via pull_request_review trigger
reserved-path-review / reserved-path-review (pull_request_review) Successful in 4s
qa-review / approved (pull_request_review) Successful in 4s
security-review / approved (pull_request_target) Approved via pull_request_review trigger
security-review / approved (pull_request_review) Successful in 11s
audit-force-merge / audit (pull_request_target) Successful in 9s
4feed3161f
Adds unit tests asserting the auto-restart guard (core#2573 / #2605):
- Self-write: caller == target workspace suppresses auto-restart.
- Platform root: target kind='platform' (the concierge) suppresses auto-restart.
- Regular workspace: auto-restart still fires (existing TestSecretsSet_AutoRestart).

Closes molecule-core#2615.

Co-Authored-By: Claude <noreply@anthropic.com>
agent-dev-a requested review from agent-reviewer-cr2 2026-06-12 04:18:25 +00:00
agent-reviewer-cr2 approved these changes 2026-06-12 04:20:05 +00:00
agent-reviewer-cr2 left a comment
Member

Approved on head 4feed3161f.

5-axis review:

Correctness: the new tests call the real SecretsHandler.Set path, including insert, callerWorkspaceID, autoRestartAllowed, and the async restart branch. They cover caller==target self-write suppression and kind=platform target suppression, while existing TestSecretsSet_AutoRestart keeps the regular-workspace restart path covered.

Robustness: the tests assert successful writes do not invoke restart by using a buffered restart channel and timeout. The platform-root test verifies the kind lookup is performed and returns platform before suppressing restart.

Security: this protects the intended no-self-secret-ops/safe-approval behavior: secret writes to the caller's own workspace or org root do not terminate the active agent/root workspace. No new auth or secret exposure is introduced.

Performance: test-only change; small added runtime from two handler unit tests.

Readability: tests are straightforward. Note: they overlap with existing #2573 tests later in the file, but the duplication is acceptable for explicit #2605 regression coverage and does not change production code.

Caveats: I could not run local Go tests because go is unavailable in this environment. PR CI was not fully green at review time due governance/advisory contexts, not this code path.

Approved on head 4feed3161f8e4d30b4c27b027d8ed170159ef6fc. 5-axis review: Correctness: the new tests call the real `SecretsHandler.Set` path, including insert, `callerWorkspaceID`, `autoRestartAllowed`, and the async restart branch. They cover caller==target self-write suppression and kind=`platform` target suppression, while existing `TestSecretsSet_AutoRestart` keeps the regular-workspace restart path covered. Robustness: the tests assert successful writes do not invoke restart by using a buffered restart channel and timeout. The platform-root test verifies the kind lookup is performed and returns `platform` before suppressing restart. Security: this protects the intended no-self-secret-ops/safe-approval behavior: secret writes to the caller's own workspace or org root do not terminate the active agent/root workspace. No new auth or secret exposure is introduced. Performance: test-only change; small added runtime from two handler unit tests. Readability: tests are straightforward. Note: they overlap with existing #2573 tests later in the file, but the duplication is acceptable for explicit #2605 regression coverage and does not change production code. Caveats: I could not run local Go tests because `go` is unavailable in this environment. PR CI was not fully green at review time due governance/advisory contexts, not this code path.
devops-engineer merged commit f6b8df67b9 into main 2026-06-12 04:25:19 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2624