Phase 2: security hardening — token user-binding, credential marker, log caps, domain enforcement (supersedes #67) - #99
Merged
Conversation
…llisions - Saddle_Approval::gate now logs a confirmed destructive execution even when the executor returns WP_Error — a partial mutation with no audit trail is worse than a noisy one. The entry carries a "FAILED after confirmation" marker plus the error message. - Saddle_Integrations::wrap surfaces genuine wrapper-name collisions via _doing_it_wrong instead of silently dropping the partner tool; its own wrappers are tracked so idempotent re-runs stay silent. Part of Phase 1 (correctness) of the 2026-07 architecture review (plugpressco/saddle-pro#39). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0133CZhoFPY6BBChDDGEa22Q
…it log caps, domain enforcement - Approval tokens are bound to the previewing user: with several agents on one site, agent A's preview can no longer be confirmed by agent B (saddle_token_user_mismatch). - Saddle-issued app passwords are recorded by UUID in user meta (saddle_issued_credentials); credential scoping, the XML-RPC block, and client revoke/rotate/list key on that immutable marker instead of the user-editable display name. Legacy prefix-named keys migrate on sight. - Saddle_Log GC caps denials (300) and executed mutations (1000) as separate filterable buckets, batch raised to 500, so denial noise can never evict change history. - Opt-in domain-drift enforcement (saddle_enforce_tier_domain, default off): write/admin abilities refuse on a migrated/cloned domain until the owner re-confirms the tier; exposed via GET/POST /settings. Phase 2 of the 2026-07 architecture review (#63). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0133CZhoFPY6BBChDDGEa22Q
# Conflicts: # includes/class-saddle-capabilities.php # includes/class-saddle-integrations.php # tests/approval-test.php
phpcs flagged it after the merge; WPCS wants the call on the left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GkZr73cqaSesHRDG89Yy8S
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
1 participant
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.
Supersedes #67, which GitHub auto-closed when its base branch (
improve/phase1-correctness, #66) was deleted on merge. Same branch, same work, brought up to date withmain. Closes #63.What
Phase 2 of the 2026-07 architecture review:
Bringing it up to date
The branch was 3.5 weeks stale and
mainhas moved a long way. Three conflicts, all resolved by keeping both sides:denial_reason()— this branch adds a domain-drift branch,main(from fix(guardrails): hold a wrapped destructive confirm to the args it previewed (#89) #90) adds a capability branch. Both belong. Ordered capability → domain → tier, mirroring the order the mergedpermission()actually checks them in (paused → capability → disabled → domain → tier).Saddle_Integrations— this branch's collision registry vsmain'starget_keys()/canonical()helpers from the wrapper-bind fix. Purely additive on both sides.tests/approval-test.php— this branch's user-mismatch test vsmain's. Additive.Also fixed a pre-existing phpcs error this branch shipped with: a non-Yoda comparison on the new user-binding check. Not from the merge — it had simply never been linted against the current ruleset.
Note on what it means alongside #90
Saddle_Approval::gate()now binds a token to action + target + bind + user. #90 added thebind(every argument the preview showed); this adds the user. Those are complementary, and together they close both halves of "a confirm that isn't the preview it claims to be".Testing
composer test— 566 tests, 1913 assertions, green (1 pre-existing skip)composer lint— 0 errorsWP_DEBUGonNo CI in this repo, so "green" means the commands above were run locally.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GkZr73cqaSesHRDG89Yy8S