[WARP•R00T] Custody Lane 3 — Signer-Plane Scaffold (Lane 3 stays OPEN)#2068
Conversation
New wallet/signing_plane.py — allowlisted, fail-closed, audited signer acquisition boundary (intents, request/grant, identity+intent authorization, repr-suppressed key material, pluggable for a future remote/KMS plane). Cut the live per-user CLOB signer (user_client._build_for_user) over to it; raw vault decrypt import removed. LOCAL plane is transitional same-host — true out-of-process/KMS isolation is OWNER_ACTION_REQUIRED. V1 stays custodial-transparent; no live order/fund/guard/Kelly/ENABLE_LIVE_TRADING change; no secrets. refs #2067 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxPHVVcpuJzkm4v6qZpheZ
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Custody Lane 3: Signer Isolation, introducing a new signing-plane boundary (wallet/signing_plane.py) to manage signer-material acquisition and cutting over the live per-user CLOB signer to this interface. The review feedback focuses on enhancing robustness and test reliability. Key recommendations include converting string UUIDs to UUID objects in SignerRequest, adding defensive checks for null values in attribution(), and using Python's ast module in tests to replace fragile substring checks when verifying that raw vault decrypt helpers are not imported or referenced directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @dataclass(frozen=True) | ||
| class SignerRequest: | ||
| """A request for a signing capability. Carries NO secret material. | ||
|
|
||
| ``target`` holds market / token / destination metadata (keys only are | ||
| surfaced in attribution). ``guard_context`` is an upstream-resolved summary | ||
| for audit — the plane does NOT re-decide live guards (that would be a hidden | ||
| bypass); guard enforcement stays in the gate/router/executor. | ||
| """ | ||
|
|
||
| user_id: UUID | ||
| intent: SigningIntent | ||
| route: str | ||
| target: dict = field(default_factory=dict) | ||
| guard_context: dict = field(default_factory=dict) |
There was a problem hiding this comment.
To prevent fragile type-check failures when user_id is passed as a string representation of a UUID (which is common when parsing external payloads or CLI arguments), consider automatically converting valid UUID strings to UUID objects in __post_init__. This ensures robust runtime behavior while maintaining strict type safety.
@dataclass(frozen=True)
class SignerRequest:
"""A request for a signing capability. Carries NO secret material.
target holds market / token / destination metadata (keys only are
surfaced in attribution). guard_context is an upstream-resolved summary
for audit - the plane does NOT re-decide live guards (that would be a hidden
bypass); guard enforcement stays in the gate/router/executor.
"""
user_id: UUID
intent: SigningIntent
route: str
target: dict = field(default_factory=dict)
guard_context: dict = field(default_factory=dict)
def __post_init__(self) -> None:
if isinstance(self.user_id, str):
try:
object.__setattr__(self, "user_id", UUID(self.user_id))
except ValueError:
pass| def attribution(self) -> dict[str, object]: | ||
| """Operator-facing attribution dict — NEVER includes secrets.""" | ||
| return { | ||
| "user_id": str(self.user_id), | ||
| "intent": self.intent.value if isinstance(self.intent, SigningIntent) else str(self.intent), | ||
| "route": self.route, | ||
| "target_keys": sorted(self.target), | ||
| "guard_context": dict(self.guard_context), | ||
| "plane_version": PLANE_VERSION, | ||
| } |
There was a problem hiding this comment.
Enforce defensive programming by safely handling cases where target or guard_context might be passed as None (e.g., from nullable variables). Directly calling sorted(self.target) or dict(self.guard_context) on None will raise a TypeError, which would suppress the audit log or crash the signing plane.
| def attribution(self) -> dict[str, object]: | |
| """Operator-facing attribution dict — NEVER includes secrets.""" | |
| return { | |
| "user_id": str(self.user_id), | |
| "intent": self.intent.value if isinstance(self.intent, SigningIntent) else str(self.intent), | |
| "route": self.route, | |
| "target_keys": sorted(self.target), | |
| "guard_context": dict(self.guard_context), | |
| "plane_version": PLANE_VERSION, | |
| } | |
| def attribution(self) -> dict[str, object]: | |
| """Operator-facing attribution dict - NEVER includes secrets.""" | |
| return { | |
| "user_id": str(self.user_id), | |
| "intent": self.intent.value if isinstance(self.intent, SigningIntent) else str(self.intent), | |
| "route": self.route, | |
| "target_keys": sorted(self.target) if self.target else [], | |
| "guard_context": dict(self.guard_context) if self.guard_context else {}, | |
| "plane_version": PLANE_VERSION, | |
| } |
References
- Maintain defensive fallback checks (such as checking for 'None' values) in critical execution paths even if current upstream logic guarantees non-null values.
| def test_live_execution_modules_do_not_decrypt_directly() -> None: | ||
| import projects.polymarket.crusaderbot.domain.execution.complete_set_exec as cse | ||
| import projects.polymarket.crusaderbot.domain.execution.live as live | ||
| import projects.polymarket.crusaderbot.domain.execution.per_user_clob as puc | ||
|
|
||
| for mod in (live, cse, puc): | ||
| src = Path(inspect.getfile(mod)).read_text(encoding="utf-8") | ||
| assert "get_decrypted_pk" not in src, ( | ||
| f"{mod.__name__} must not call the raw vault decrypt helper directly" | ||
| ) |
There was a problem hiding this comment.
Using a simple substring check like "get_decrypted_pk" not in src is fragile because any developer adding a comment or docstring referencing get_decrypted_pk in the future will break this test. Consider using Python's ast module to parse the source code and robustly verify that get_decrypted_pk is neither imported nor called, completely ignoring comments and docstrings.
| def test_live_execution_modules_do_not_decrypt_directly() -> None: | |
| import projects.polymarket.crusaderbot.domain.execution.complete_set_exec as cse | |
| import projects.polymarket.crusaderbot.domain.execution.live as live | |
| import projects.polymarket.crusaderbot.domain.execution.per_user_clob as puc | |
| for mod in (live, cse, puc): | |
| src = Path(inspect.getfile(mod)).read_text(encoding="utf-8") | |
| assert "get_decrypted_pk" not in src, ( | |
| f"{mod.__name__} must not call the raw vault decrypt helper directly" | |
| ) | |
| def test_live_execution_modules_do_not_decrypt_directly() -> None: | |
| import ast | |
| import projects.polymarket.crusaderbot.domain.execution.complete_set_exec as cse | |
| import projects.polymarket.crusaderbot.domain.execution.live as live | |
| import projects.polymarket.crusaderbot.domain.execution.per_user_clob as puc | |
| for mod in (live, cse, puc): | |
| src = Path(inspect.getfile(mod)).read_text(encoding="utf-8") | |
| tree = ast.parse(src) | |
| for node in ast.walk(tree): | |
| if isinstance(node, ast.Name) and node.id == "get_decrypted_pk": | |
| raise AssertionError(f"{mod.__name__} references get_decrypted_pk directly") | |
| if isinstance(node, ast.ImportFrom) and any(alias.name == "get_decrypted_pk" for alias in node.names): | |
| raise AssertionError(f"{mod.__name__} imports get_decrypted_pk directly") |
References
- In Python test files, absolute imports starting with the project's root package (e.g., 'projects.polymarket.crusaderbot.*') are acceptable and should be used for consistency if that is the repository's convention.
| def test_user_client_uses_signing_plane_not_raw_vault() -> None: | ||
| from projects.polymarket.crusaderbot.integrations.clob import user_client | ||
|
|
||
| src = Path(inspect.getfile(user_client)).read_text(encoding="utf-8") | ||
| assert "signing_plane" in src and "acquire_signer" in src, ( | ||
| "per-user build must acquire its signer through the signing plane" | ||
| ) | ||
| assert "from ...wallet.vault import get_decrypted_pk" not in src, ( | ||
| "per-user build must NOT import the raw vault decrypt helper directly" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Similarly, checking for raw string imports like "from ...wallet.vault import get_decrypted_pk" not in src is prone to false positives if the import is mentioned in comments or docstrings. Using an AST-based check is much more robust and maintainable.
def test_user_client_uses_signing_plane_not_raw_vault() -> None:
import ast
from projects.polymarket.crusaderbot.integrations.clob import user_client
src = Path(inspect.getfile(user_client)).read_text(encoding="utf-8")
tree = ast.parse(src)
has_signing_plane = False
has_acquire_signer = False
for node in ast.walk(tree):
if isinstance(node, ast.Name):
if node.id == "signing_plane":
has_signing_plane = True
elif node.id == "acquire_signer":
has_acquire_signer = True
elif node.id == "get_decrypted_pk":
raise AssertionError("user_client references get_decrypted_pk directly")
elif isinstance(node, ast.ImportFrom):
if any(alias.name == "get_decrypted_pk" for alias in node.names):
raise AssertionError("user_client imports get_decrypted_pk directly")
assert has_signing_plane and has_acquire_signer, (
"per-user build must acquire its signer through the signing plane"
)References
- In Python test files, absolute imports starting with the project's root package (e.g., 'projects.polymarket.crusaderbot.*') are acceptable and should be used for consistency if that is the repository's convention.
There was a problem hiding this comment.
Adopted the AST approach (c0ce0ee) — agreed the substring check is fragile. One correction: the suggested snippet checks acquire_signer and signing_plane as ast.Name nodes, but in this module acquire_signer is an attribute access (get_signing_plane().acquire_signer(...) → ast.Attribute) and signing_plane only appears in the ImportFrom.module (from ...wallet.signing_plane import ...) — so the as-written version would assert-fail. I applied a corrected helper that inspects ast.Name, ast.Attribute, and ast.ImportFrom names/modules: it denies any get_decrypted_pk import/name/attr and confirms the plane is used via an ImportFrom module ending in signing_plane or an acquire_signer attribute. Same fix applied to test_live_execution_modules_do_not_decrypt_directly. The UUID-coercion and None-safe attribution() suggestions were also applied.
Generated by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 546ac884e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RECOVERY = "recovery" | ||
|
|
||
|
|
||
| _ALLOWLISTED_INTENTS: frozenset[SigningIntent] = frozenset(SigningIntent) |
There was a problem hiding this comment.
In a live process, any code that can import this new plane can request ONCHAIN_TRANSFER, ONCHAIN_SWEEP, SHARED_CLOB_SIGNER, etc. with any non-empty route and _authorize will pass because the allowlist is frozenset(SigningIntent); LocalSigningPlane then decrypts and returns the user's key. I checked the wired caller is only user_client._build_for_user with PER_USER_CLOB_SIGNER, while the report/WORKTODO leave shared and on-chain cutovers for later, so pre-authorizing every declared intent weakens the fail-closed custody boundary. Keep the allowlist to currently implemented (intent, route) pairs and add entries only as those lanes land.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Strong catch — applied in c0ce0ee. The allowlist now splits into _DECLARED_INTENTS (the stable enum surface future lanes target) and _IMPLEMENTED_INTENTS = {PER_USER_CLOB_SIGNER} (only what's wired this lane). _authorize denies any declared-but-unwired intent (ONCHAIN_TRANSFER, SHARED_CLOB_SIGNER, REDEMPTION, CLOSE_UNWIND, …) with reason_code="intent_not_enabled" before any decrypt, so no importer can obtain a user's key under an unimplemented intent. Lane 4+ add their (intent) to the implemented set as each callsite cutover lands. New test test_denies_declared_but_unimplemented_intent covers it.
Generated by Claude Code
Codex P2: the signer-grant allowlist was every declared intent, so any importer could request a user's key under an unwired intent. Restrict grants to _IMPLEMENTED_INTENTS (per_user_clob_signer only this lane); declared-but-unwired intents are intent_not_enabled-denied until their callsite cutover lands. Gemini: coerce valid UUID strings in SignerRequest, None-safe attribution(), and AST-based architectural tests (corrected — acquire_signer is an attribute, not a Name). refs #2067 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxPHVVcpuJzkm4v6qZpheZ
|
DECISION: HOLD MAJOR signer/capital/custody gate. CI is green and the local signing-plane scaffold is mechanically clean, but this PR cannot merge as written because the PR scope/closure overclaims Lane 3. Blockers:
Acceptable fix paths: A. Narrow this PR honestly to a scaffold PR: remove B. Complete the Lane 3 contract in this PR: keep No merge until one of those paths is applied. No runtime/live/funds issue found; this is a custody-boundary correctness blocker. |
|
CMD PROCESS DIRECTIVE — WARP•R00T CONTINUATION Boss directive received: all required fixes, scope corrections, or follow-up instructions must be handled directly in this PR thread. Do not wait for side-channel clarification when the gate finding is clear. For this PR, proceed with one of the two fix paths already posted. CMD recommendation: use Fix Path A unless you can truly complete signer isolation in-repo without overclaiming. Recommended current action:
After a clean merge, CMD will include the next task issue link in the merge comment so WARP•R00T can continue directly into the next work lane without a fresh owner briefing. Expected next task after scaffold merge, unless true Lane 3 is completed here: Goal for 3B: make the main app stop receiving raw key material, move signing out of process or behind an owner-approved KMS/HSM boundary, and cut over remaining shared/complete-set/on-chain signer callsites or fail-close them under the signing plane. |
|
CMD OWNER-PATH DECISION Proceed with: A + fold in achievable hardening. Meaning:
Required changes before CMD can merge:
After this lands cleanly with CI green, CMD can merge the scaffold PR and create/link the next continuation task in the merge comment: Goal for 3B: make the main app stop receiving raw key material, move signing out of process or behind owner-approved KMS/HSM boundary, and complete remaining signer callsite cutovers. |
CMD HOLD on #2068: re-scope to a signer-plane scaffold (Refs #2067, NOT Closes; verdict OWNER_ACTION_REQUIRED; Lane 3 stays OPEN; state/report/WORKTODO/ CHANGELOG say true isolation is NOT complete — main app still receives the key via SignerGrant.material). Folded in achievable Path-B hardening: implemented intents require non-empty target+guard_context (missing_target/ missing_guard_context denials); the shared_transitional + complete-set live signer paths now route through the plane and FAIL CLOSED (SHARED_CLOB_SIGNER not enabled) so ALLOW_SHARED_LIVE_EXECUTION can no longer obtain a signer outside the boundary. Tests updated accordingly. No live order/fund/guard/Kelly/ ENABLE_LIVE_TRADING change; no secrets. refs #2067 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxPHVVcpuJzkm4v6qZpheZ
bayuewalker
changed the title
|
WARP•R00T — CMD HOLD directive applied (path: A + fold in achievable hardening). Pushed 1. Scope / closure honesty
2. Enforce target + guard context — 3. Remove shared / complete-set bypass — both the 4. Hard constraints intact — no live order / fund / DB write / Fly secret / CI re-running on Generated by Claude Code |
The Lane 3 shared signer fence fires before get_clob_client, so the two TestExceptionClassification cases (real-CLOB shared open, get_clob_client patched to raise) now hit shared_signer_not_available instead of the get_clob_client construction-error reclassification. Updated both to assert the Lane 3 fail-closed reality (LivePreSubmitError shared_signer_not_available, get_clob_client never reached) — the router-safe pre-submit property is preserved; the ClobConfigError/ClobAuthError reclassification remains in code for the per-user/non-real paths. refs #2067 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxPHVVcpuJzkm4v6qZpheZ
|
DECISION: HOLD — CI FAILURE AFTER SYNC The scope fix is directionally correct now:
Current blocker: CI is red. Failing check:
Passing checks:
Required action:
CMD will re-gate after the next synchronize event. No merge while CI is red. |
|
WARP•R00T — CI red resolved. The Root cause + fix: the 2 failures were Scope honesty intact (unchanged): PR is scaffold/foundation, Ready for CMD re-gate on the Generated by Claude Code |
|
DECISION: MERGE — executed. Merged PR #2068 as a scaffold/foundation PR, NOT completed Lane 3. Merge SHA: Gate summary:
GitHub auto-closed #2067 on merge despite the PR body saying Next continuation issue for WARP•R00T: |
Custody Lane 3 — Signer-Plane Scaffold (MAJOR / owner-routed WARP•R00T)
Refs #2067— this PR does NOT close #2067. It is a signer-plane scaffold / foundation, NOT completed signer isolation. Verdict:OWNER_ACTION_REQUIRED. Lane 3 / #2067 remains OPEN for the continuation lane 3B — Remote/KMS Signing Plane Cutover.Scope honesty (CMD HOLD on #2068 — applied)
SignerGrant.material. That is a reduced, authorized, audited call surface — not signer isolation.PROJECT_STATE/WORKTODO/CHANGELOG/ report all state Lane 3 is NOT complete and [WARP•R00T] Custody Lane 3 — Signer Isolation #2067 stays open.What this scaffold delivers
wallet/signing_plane.py— narrow, fail-closed, audited signer-acquisition boundary. Grant allowlist restricted to implemented intents (per_user_clob_signeronly; every other declared intent isintent_not_enabled-denied). Implemented intents require non-emptytarget+guard_context(missing_target/missing_guard_contextdenials). Key material isrepr-suppressed and never logged/audited. Pluggable (set_signing_plane) so a future remote/KMS plane drops in with no caller change.user_client._build_for_useracquires its signer through the plane (with honest build-contexttarget/guard_context); the directwallet.vault.get_decrypted_pkimport is removed.Fold-in hardening (achievable Path-B, per CMD directive)
live.executeshared_transitional path andcomplete_set_exec.book_complete_set_liveshared path now route their signer through the plane withSHARED_CLOB_SIGNER— which is declared-but-not-enabled — so they fail closed (shared_signer_not_available→ paper /live_blocked_shared_signer_not_available→ skipped).ALLOW_SHARED_LIVE_EXECUTIONcan no longer obtain a signer outside the boundary.Safety posture (unchanged constraints)
No live order, no fund movement, no DB write, no Fly secret change, no
ENABLE_LIVE_TRADINGflip; no live-guard /USE_REAL_CLOB/ kill-switch / CLOB-readiness / balance-cap / idempotency / dedup bypass; no Kelly/sizing/risk-control change (full Kelly forbidden); no secrets or real signature fixtures in code/tests/reports/state/logs.Tests & CI
tests/test_custody_lane3_signing_plane.py— authorization (identity / declared-intent / intent_not_enabled / route / missing_target / missing_guard_context), fail-closed on signer-source error, no-material empty grant, secret-hygiene, pluggability, AST-based architectural cutover checks.test_live_per_user_wiring.py+test_complete_set_live.pyupdated: shared opt-in now fails closed at the plane.ruff+py_compileclean. Heavy-dependency suite (CI-onlypy_clob_client_v2) gates in CI.Verdict:
OWNER_ACTION_REQUIRED. Next: Lane 3B — Remote/KMS Signing Plane Cutover (#2067 stays open).Report:
projects/polymarket/crusaderbot/reports/root/custody-lane3-signer-isolation.md🤖 Generated with Claude Code
https://claude.ai/code/session_01BxPHVVcpuJzkm4v6qZpheZ