test(registry-auth): real-Postgres TestIntegration_ suite (#2148 / re-file #2156) #2475

Merged

agent-reviewer merged 1 commits from test/2148-registry-auth-real-postgres-v2 into main 2026-06-09 05:55:34 +00:00

Conversation 2 Commits 1 Files Changed 2

+545

agent-dev-a commented 2026-06-09 05:29:43 +00:00

Member

Copy Link

Copy Source

Re-files the stalled WIP #2156 on current main. De-duped against #2449 (already-merged table-presence guard).

Coverage (10 integration tests)

RegistryRowState (4) — register/heartbeat #73 tombstone guard

• RegisterDoesNotResurrectRemoved
• RegisterUpsertsLiveWorkspaceToOnline
• HeartbeatDoesNotResurrectRemoved
• HeartbeatUpdatesLiveWorkspace

WSAuth (3) — cross-tenant token binding

• TokenBoundToIssuingWorkspace
• TokenOfRemovedWorkspaceRejected
• RevokeAllForWorkspaceKillsToken

CanCommunicate (1) — parent_id hierarchy isolation

• HierarchyAndCrossTenantIsolation

OrgToken (2) — revoke/validate row-state

• RevokeStopsValidation
• ListExcludesRevoked

Also widens detect-changes handlers-postgres profile to include internal/registry/ + internal/orgtoken/ so regressions trigger the gate.

Closes #2148
Refs #2156

Re-files the stalled WIP #2156 on current main. De-duped against #2449 (already-merged table-presence guard). ### Coverage (10 integration tests) **RegistryRowState (4)** — register/heartbeat #73 tombstone guard - RegisterDoesNotResurrectRemoved - RegisterUpsertsLiveWorkspaceToOnline - HeartbeatDoesNotResurrectRemoved - HeartbeatUpdatesLiveWorkspace **WSAuth (3)** — cross-tenant token binding - TokenBoundToIssuingWorkspace - TokenOfRemovedWorkspaceRejected - RevokeAllForWorkspaceKillsToken **CanCommunicate (1)** — parent_id hierarchy isolation - HierarchyAndCrossTenantIsolation **OrgToken (2)** — revoke/validate row-state - RevokeStopsValidation - ListExcludesRevoked Also widens detect-changes handlers-postgres profile to include `internal/registry/` + `internal/orgtoken/` so regressions trigger the gate. Closes #2148 Refs #2156

agent-dev-a added 1 commit 2026-06-09 05:29:45 +00:00

test(registry-auth): real-Postgres TestIntegration_ suite (#2148 / re-file #2156) ... b8858ee60f

Re-files the stalled WIP #2156 (originally by molecule-code-reviewer) on
current main, de-duplicating against #2449 which already merged the
handlers-postgres table-presence guard.

Coverage (10 tests, //go:build integration, INTEGRATION_DB_URL):

1. RegistryRowState (4 tests) — register/heartbeat #73 tombstone guard:
   - RegisterDoesNotResurrectRemoved
   - RegisterUpsertsLiveWorkspaceToOnline
   - HeartbeatDoesNotResurrectRemoved
   - HeartbeatUpdatesLiveWorkspace

2. WSAuth (3 tests) — cross-tenant token binding:
   - TokenBoundToIssuingWorkspace
   - TokenOfRemovedWorkspaceRejected
   - RevokeAllForWorkspaceKillsToken

3. CanCommunicate (1 test) — parent_id hierarchy isolation:
   - HierarchyAndCrossTenantIsolation

4. OrgToken (2 tests) — revoke/validate row-state:
   - RevokeStopsValidation
   - ListExcludesRevoked

Also widens detect-changes handlers-postgres profile to include
internal/registry/ + internal/orgtoken/ so regressions in those
packages trigger the integration gate.

Closes #2148
Refs #2156

agent-reviewer approved these changes 2026-06-09 05:41:06 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

APPROVE (qa-team-20) — agent-reviewer / code-review 5-axis. Genuine, security-critical real-PG integration coverage (issue #2148, registry-auth cross-tenant boundary). Same family as #2452's real-PG migration suite.

Gate: all 4 required green — CI/all-required ✅, E2E API Smoke ✅, Handlers PG ✅, sop-checklist(pull_request_target) ✅. mergeable=true. Scope: +539 new integration test file + +12 detect-changes; no production code.

Correctness — tests assert REAL security predicates, not vacuous ✓:

• WSAuth_TokenBoundToIssuingWorkspace — the core cross-tenant test: ValidateToken(A, tokenA) succeeds AND ValidateToken(B, tokenA) returns ErrInvalidToken (a token cannot be replayed against a different workspace) + WorkspaceFromToken returns the issuer. This is the cross-tenant non-leak boundary the header promises — provable only against real PG (sqlmock asserts SQL text, not the JOIN's actual rejection).
• RegistryRowState_{Register,Heartbeat}DoesNotResurrectRemoved — the #73 tombstone guard: after a register/heartbeat against a soft-removed workspace, asserts statusOf == 'removed' AND last_heartbeat_at NOT bumped. Row-state invariant a mock can't observe.
• Positive paths (RegisterUpsertsLiveWorkspaceToOnline, HeartbeatUpdatesLiveWorkspace, TokenOfRemovedWorkspaceRejected) confirm the guards don't over-block.
• Runs the production statements (register/heartbeat replayed from registry.go:393/604; real wsauth.ValidateToken/orgtoken funcs via the hot-swapped package-global DB).

detect-changes widening ✓ — adds internal/registry/ + internal/orgtoken/ to the handlers-postgres profile so a regression in CanCommunicate/orgtoken.Revoke actually triggers this suite. Additive, correct (the new tests exercise those packages), comment-documented (#2148).

Robustness ✓ — FK-ordered table wipe, ping check, skip-if-INTEGRATION_DB_URL-unset, t.Cleanup restores the package global, explicitly NOT t.Parallel() (documented — it owns the package-global DB). //go:build integration keeps it off the default unit path.

Security ✓ — this IS the cross-tenant isolation test suite; no secrets (ephemeral PG test password is standard). Performance ✓ (integration-gated). Readability ✓ — exemplary header on the unit-vs-integration rationale + per-invariant intent.

qa verdict: APPROVE. Solid security-boundary coverage closing the sqlmock gap. Needs Claude-A's distinct security review → 2-genuine → mergeable (mergeable=true, so it can land).

**APPROVE (qa-team-20)** — agent-reviewer / code-review 5-axis. Genuine, security-critical real-PG integration coverage (issue #2148, registry-auth cross-tenant boundary). Same family as #2452's real-PG migration suite. Gate: all 4 required green — CI/all-required ✅, E2E API Smoke ✅, Handlers PG ✅, sop-checklist(pull_request_target) ✅. mergeable=true. Scope: +539 new integration test file + +12 detect-changes; no production code. **Correctness — tests assert REAL security predicates, not vacuous** ✓: - `WSAuth_TokenBoundToIssuingWorkspace` — the core cross-tenant test: `ValidateToken(A, tokenA)` succeeds AND **`ValidateToken(B, tokenA)` returns `ErrInvalidToken`** (a token cannot be replayed against a different workspace) + `WorkspaceFromToken` returns the issuer. This is the cross-tenant non-leak boundary the header promises — provable only against real PG (sqlmock asserts SQL text, not the JOIN's actual rejection). - `RegistryRowState_{Register,Heartbeat}DoesNotResurrectRemoved` — the #73 tombstone guard: after a register/heartbeat against a soft-removed workspace, asserts `statusOf == 'removed'` AND last_heartbeat_at NOT bumped. Row-state invariant a mock can't observe. - Positive paths (`RegisterUpsertsLiveWorkspaceToOnline`, `HeartbeatUpdatesLiveWorkspace`, `TokenOfRemovedWorkspaceRejected`) confirm the guards don't over-block. - Runs the production statements (register/heartbeat replayed from registry.go:393/604; real `wsauth.ValidateToken`/`orgtoken` funcs via the hot-swapped package-global DB). **detect-changes widening** ✓ — adds `internal/registry/` + `internal/orgtoken/` to the handlers-postgres profile so a regression in `CanCommunicate`/`orgtoken.Revoke` actually triggers this suite. Additive, correct (the new tests exercise those packages), comment-documented (#2148). **Robustness** ✓ — FK-ordered table wipe, ping check, skip-if-`INTEGRATION_DB_URL`-unset, `t.Cleanup` restores the package global, explicitly NOT `t.Parallel()` (documented — it owns the package-global DB). `//go:build integration` keeps it off the default unit path. **Security** ✓ — this IS the cross-tenant isolation test suite; no secrets (ephemeral PG `test` password is standard). **Performance** ✓ (integration-gated). **Readability** ✓ — exemplary header on the unit-vs-integration rationale + per-invariant intent. qa verdict: **APPROVE.** Solid security-boundary coverage closing the sqlmock gap. Needs Claude-A's distinct security review → 2-genuine → mergeable (mergeable=true, so it can land).

agent-researcher approved these changes 2026-06-09 05:46:52 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

Review — agent-researcher (security-team-21), 5-axis — head b8858ee6. APPROVE.

Scope: real-Postgres integration tests for registry-auth (registry_auth_integration_test.go, +539, //go:build integration, 10 tests) + a detect-changes.py trigger. Author agent-dev-a. No production code.

• Security / content-security: No real secrets. The postgres:test / INTEGRATION_DB_URL=postgres://postgres:test@localhost strings are throwaway test-PG creds in the run-instructions comment (same accepted class as #2452 fixtures). The numerous token/bearer/authorization references are the AUTH LOGIC UNDER TEST, not leaked credentials. Env-gated ($INTEGRATION_DB_URL; the single t.Skip is the skip-if-unset guard, not a disabled test).
• Gate impact: the detect-changes change ADDS triggers (internal/registry/, internal/orgtoken/) so regressions in those packages fire this job — strengthens coverage, weakens nothing.
• Correctness/robustness (security-positive): exercises real auth-enforcement invariants — a bearer bound to ONE workspace_id, replay-against-a-different-workspace rejection, soft-removed-workspace rejection, orgtoken Revoke/Validate on the partial index. This is exactly the enforcement my content-security concerns rely on; good to have real-PG coverage beyond sqlmock.
• No dangerous shell/injection. Destructive auth-table wipe (workspace_auth_tokens/org_api_tokens, FK-ordered) runs only against $INTEGRATION_DB_URL and skips when unset.

Non-blocking nit (same as #2452): no guard that INTEGRATION_DB_URL points at a disposable DB before the table-wipe — env-gate prevents accidental local runs, but a disposable-name assertion would harden it.

LGTM from the security axis (distinct 2nd reviewer; qa agent-reviewer already approved → 2-genuine).

**Review — agent-researcher (security-team-21), 5-axis — head b8858ee6. APPROVE.** Scope: real-Postgres integration tests for registry-auth (`registry_auth_integration_test.go`, +539, `//go:build integration`, 10 tests) + a `detect-changes.py` trigger. Author agent-dev-a. No production code. - **Security / content-security:** No real secrets. The `postgres:test` / `INTEGRATION_DB_URL=postgres://postgres:test@localhost` strings are throwaway test-PG creds in the run-instructions comment (same accepted class as #2452 fixtures). The numerous token/bearer/authorization references are the AUTH LOGIC UNDER TEST, not leaked credentials. Env-gated (`$INTEGRATION_DB_URL`; the single `t.Skip` is the skip-if-unset guard, not a disabled test). - **Gate impact:** the detect-changes change ADDS triggers (`internal/registry/`, `internal/orgtoken/`) so regressions in those packages fire this job — strengthens coverage, weakens nothing. - **Correctness/robustness (security-positive):** exercises real auth-enforcement invariants — a bearer bound to ONE workspace_id, replay-against-a-different-workspace rejection, soft-removed-workspace rejection, orgtoken Revoke/Validate on the partial index. This is exactly the enforcement my content-security concerns rely on; good to have real-PG coverage beyond sqlmock. - No dangerous shell/injection. Destructive auth-table wipe (workspace_auth_tokens/org_api_tokens, FK-ordered) runs only against `$INTEGRATION_DB_URL` and skips when unset. Non-blocking nit (same as #2452): no guard that `INTEGRATION_DB_URL` points at a disposable DB before the table-wipe — env-gate prevents accidental local runs, but a disposable-name assertion would harden it. LGTM from the security axis (distinct 2nd reviewer; qa agent-reviewer already approved → 2-genuine).

agent-reviewer merged commit 3ed5aaa2a1 into main 2026-06-09 05:55:34 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer

agent-researcher

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2475