From 2a7019b0fa02587446e55af3838603d1c6924dba Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 7 Sep 2026 12:00:48 +0100 Subject: [PATCH] Use typed Proxmox runners and preserve cross-clock action evidence Proxmox planning and dispatch now require a unique credential-admitted typed runner with durable receipts. Development authentication preserves explicit bearer identity so runner activation keeps its tenant and credential scope. Preserve observer and receiver timestamps from their separate clocks instead of rejecting or rewriting valid evidence. Keep completed execution separate from stale or inconclusive verification, and label independent observations accurately in action reviews. Verified with targeted race suites, action-review tests and frontend build, plus a real Assistant start plan and approved VM110 start/stop with independent Proxmox confirmation. Final action reviews passed Playwright at 1440, 900 and 390 pixels, including retained completed, rejected and expired history. --- .../PATROL_ASSISTANT_CUSTOMER_JOURNEY.md | 204 +++++++++++++++ docs/release-control/v6/internal/status.json | 18 +- .../v6/internal/subsystems/agent-lifecycle.md | 17 +- .../v6/internal/subsystems/api-contracts.md | 29 ++- .../subsystems/frontend-primitives.md | 6 + .../subsystems/performance-and-scalability.md | 8 +- .../v6/internal/subsystems/registry.json | 1 + .../internal/subsystems/security-privacy.md | 6 + .../internal/subsystems/storage-recovery.md | 19 +- .../internal/subsystems/unified-resources.md | 15 ++ frontend-modern/browser-verification.json | 53 ++-- .../features/actions/ActionDecisionPacket.tsx | 2 +- .../__tests__/ActionDecisionPacket.test.tsx | 27 +- internal/agentexec/server.go | 50 +++- internal/agentexec/server_test.go | 48 ++++ internal/api/admin_bypass_router_test.go | 49 ++++ internal/api/auth.go | 17 +- internal/api/contract_test.go | 48 +--- .../api/docker_container_action_result.go | 26 +- .../docker_container_action_result_test.go | 12 +- internal/api/host_apt_action_result.go | 11 +- internal/api/host_apt_action_result_test.go | 6 +- internal/api/proxmox_guest_action_executor.go | 193 ++++---------- .../api/proxmox_guest_action_executor_test.go | 241 ++++++++++++++---- internal/api/proxmox_guest_action_result.go | 16 +- .../api/proxmox_patrol_integration_test.go | 8 +- internal/api/router.go | 2 +- internal/api/security_test.go | 24 ++ internal/unifiedresources/action_result_v2.go | 6 +- .../unifiedresources/action_result_v2_test.go | 28 ++ 30 files changed, 828 insertions(+), 362 deletions(-) diff --git a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md index 1e180397b..b753c152f 100644 --- a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md +++ b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md @@ -2569,3 +2569,207 @@ expansion, reachable controls and dialog placement. Reload, keyboard expansion, Escape and close checks passed. The current browser receipt supersedes the earlier artifact binding for this slice. Missing-access continuity, VM dispatch and Assistant orchestration failures remain separate open qualification defects. + +### Typed Proxmox runner qualification, 2026-09-07 + +The previous filesystem/history slice was committed as +`3a4a3fd62bb8d36660f6f0756ed8b58338f39d83` and pushed to PR #1951 with +auto-merge enabled. Exact staged pre-commit and pre-push hooks passed on the +non-root worker against tree `90e4b6e3e59d6bcd514f0f5af1ff3d69bc8adb8a`. +All required checks passed and PR #1951 merged as +`09ab5c2d0ae6e02fcc5280853782a8142646e40f`. The advisory benchmark reported +four NormalizeSegment microbenchmark regressions against unchanged source. +That observation is not a failure of the required feature qualification. + +The VM110 execution error was recovered from the backend log: the server +rejected the generic command session because Proxmox guest lifecycle requires +a typed action runner. This is separate from the earlier SQLite plan-write +failure. Planning and binding previously accepted generic connection presence, +while the real execution path checked the credential-bound role. Existing +Proxmox API tests exercised a shell fallback instead of that real typed path. + +The current source resolves a unique, currently admitted typed runner for the +owning node and tenant. It rejects legacy, revoked, pending, fenced and +ambiguous sessions and requires durable receipt support before plan persistence. +The dispatch guard checks the actual selected connection. The API shell +fallback is removed and its tests use typed payloads. Mutation completion no +longer substitutes for readback evidence, and a status-only reboot read remains +inconclusive without independent uptime evidence. The full agentexec race suite +and focused Proxmox API race cases pass on the worker (9.062 and 3.295 seconds). + +The rebuilt development server artifact is +`b4aa4f9c47e3740ac6c409e91d9ab84beb25c0dff3b8641ce04872929df58004`, with +native Linux runner +`42ab7aa4f7c0c2f825586d5ae536254b8282924cb056cd4d65fce53a19c43b44`. +The live VM110 qualification is in progress. Two fixture setup attempts stopped +before runner registration: an executable placed on Delly's noexec `/run`, then +a temporary service name that did not satisfy the canonical +`pulse-agent-runner.service` containment dependency. Both attempts restored the +stopped VM, revoked the temporary credential and removed the private tunnel. +These are test setup failures, not successful execution proofs. Runtime +restrictions were retained. No production agent was changed. + +With the canonical service name, the runner passed containment and WebSocket +registration, then activation returned HTTP403. A single isolated diagnostic +request recovered the exact reason: `Action runner bearer credential required`. +The managed development runtime enables admin bypass, which returned a generic +admin context before extracting the supplied bearer token. The exact-token +activation endpoint correctly refused that missing identity. The shared auth +path now preserves normal validation and scopes whenever explicit credentials +are present. This does not weaken activation or change production auth mode. +The third fixture and diagnostic credentials were revoked and all temporary +services and tunnels removed. Final rebuilt qualification remains pending. + +The auth regression group passed with race detection (7.863 seconds), including +explicit-token development bypass, runner issuance/activation and Basic auth. +Six direct Basic-auth fixtures now reset their shared lockout state. The two +successful-auth fixtures also initialize and close their own session store. +Earlier grouped failures came from process-global lockout contamination, and +isolated successful-auth cases exposed missing session initialization. Runtime +lockout behavior was not changed. + +Artifact `c1233ccf5e1fc7872f9e30c1e372019d8037468603ad92fdd62f0d0d9882fa0b` +registered and activated the exact runner credential successfully. Restarting +the dev server had removed the previous live monitored-host identity, so the +qualification fixture now starts a separate monitoring-only collector with a +fresh report and report/config scopes, then issues the separate action-runner +credential. Stale retained resource metadata does not establish that prerequisite. +Both temporary services and credentials are removed at cleanup. + +The first full typed run produced Gemini plan +`act_9c97005e3eca1a957bb172e7d123fdda` in session +`c6627f40-8087-4d70-bc79-52cfd86560fb` for exactly VM110 on Delly. It consumed +51,259 input and 861 output tokens and reported $0.041673. Canonical approval +and execution completed the start, with an independent Proxmox API observation +of running state and a separate SSH `qm status` confirmation. The restoration +stop `act_2ab4ded219d68854368b445f08234939` likewise completed and independently +confirmed stopped state. All temporary services, credentials and tunnels were +removed, the original production agent PID1565 was retained and control returned +to read-only. + +That run exposed a shared evidence label saying `Agent observed` for independent +Proxmox API evidence. The shared decision packet now uses `Observed` while +retaining the named observer and separate receipt time. Nine existing decision +packet tests and the worker production frontend build passed. The replacement +bundled artifact is +`7504f38902ce90edd9dbf961a78c2ab4fc7a21ac232e9447c984ff89ca79c525`. +A final repeated lifecycle and browser matrix is in progress. The earlier stop +browser pass was interrupted by login rate limiting because the fixture signed +in per viewport. Browser contexts now reuse their authenticated session, with +no product auth or rate-limit relaxation. The redundant read/verification turn +following a planned Assistant control remains an open orchestration defect. + +The final-label repeat invalidated a claim of reliable repeated VM execution. +Plan `act_c59346a6cfcddaee4e70f0870e2ec16e` was approved, but result projection +returned HTTP500 with `normalize Proxmox agent readback evidence: invalid action +evidence: receivedAt predates observedAt`. The durable audit remained executing +with a receipt-pending attempt. The fixture cleanup restored stopped state and +removed both temporary services, credentials and tunnel. The exact action was +then force-failed with inconclusive execution truth through the operator API. +It was not retried or relabelled successful. + +A bounded SSH time sample measured Delly between 44.95 and 205.19 milliseconds +ahead of the Mac, without changing either clock. The shared evidence normalizer +incorrectly required receiver wall time to follow observer wall time across +machines. Canonical evidence now preserves both original UTC timestamps and +binds them into the digest. Capability-specific freshness checks remain required. +Docker and host-update projectors no longer clamp a valid observation to receipt +time. The Proxmox path captures the actual receipt boundary before its independent +observation and bounds agent readback freshness separately. Stale or excessive +clock skew leaves verification inconclusive without discarding completed execution. +The final shared action-result/evidence race selection passed in 2.339 seconds. +The final API cross-clock, typed Proxmox, Docker/Proxmox contract and explicit +authentication selection passed with race detection in 9.287 seconds on +`pulse-dev`. This followed the independent release preflight through the normal +allocator, without a Mac heavy-test fallback or lock bypass. + +The final enterprise development binary has SHA256 +`c53f2e54e7ea510fc3abfbfbe164e74e363d3b1560aaa36a62df9fb1aaaf2c7d`. +Its source manifest matches the final runtime and frontend files. Gemini session +`22bcd9ee-52ba-4733-8b6c-5f1e8d2bb32f` prepared one exact VM110 start plan, +using 88,539 input tokens and 1,106 output tokens at $0.07055175. Start action +`act_9ec8d575b777a5f8fc70561376a52381` and restoration stop action +`act_3384576233bf55aa3182537ad02d81d4` both completed with independently +confirmed Proxmox state. Their retained native receipts are terminal, report +completed mutation and preserve original running/stopped readback timestamps. + +Playwright passed pending and completed review states for both actions at +1440x1000, 900x1000 and 390x1000. Final pixels were inspected, including expanded +independent evidence, keyboard focus, nested scrolling and reachable footers. +Keyboard disclosures, Escape, explicit close, exact-link reopen and reload all +passed. Independent evidence now says `Observed`, while the delivery record's +agent-specific timestamp retains its accurate label. The same final build passed +retained completed, rejected and expired Docker action review smoke at all three +widths. Approvals and execution used the exact plan hashes through the canonical +API. Browser writes were limited to login. + +Cleanup restored stopped VM110, preserved production agent PID1565, removed both +temporary services and their private directories, revoked both credentials, +closed the tunnel and restored read-only control. A fresh plan then returned +HTTP409 `action_runner_unavailable`, matching stopped-resource readiness. +Worker receipts are in workspace `tmp/patrol-runner-readiness/runtime/clock-final-build/`. +Live, native receipt and browser artifacts are in +`tmp/patrol-runner-readiness/{vm-transaction.json,browser/}`. This qualifies the +named typed runner and clock-fix slice, not the full redesign or autonomous modes. +The action-review regression fixture now includes a real independent observation +with distinct observer and receiver clocks, rather than an empty evidence list. + +### Next canonical planning boundary + +The remaining missing-access defect is broader than an absent prompt hint. +`ProposalCatalog` returns static capability definitions. `ProposalCapture` +validates and holds a proposal, but the actual broker plans it only after the +model has finished. `ProposalCapture.Outcome` also converts any unsuccessful +proposal attempt followed by a no-action conclusion into an investigation error. +Those rules prevent the model from incorporating canonical planning refusal and +can override a valid decision to stop. Assistant's separate verification FSM +then mistakes its own successful plan submission for executed infrastructure. + +The next implementation should put canonical planning results into the model's +actual tool turn. Separate broker planning from policy-authorized execution, +using the existing action lifecycle for both. A proposal tool should return the +persisted plan reference or exact planning refusal before the model concludes. +Current availability belongs beside the capability schema, with the canonical +planner rechecking it. Keep the trusted finding/investigation/proposal identity, +non-sensitive parameters, tenant authority and idempotency. Canonical action +identity must replace the duplicate request-local proposal fingerprint/state +machine. A later provider error must not erase an already persisted plan. +Policy-authorized execution remains a separate lifecycle transition with its +current authority recheck, and its eventual outcome is reconciled through the +same linked action. The investigation profile must not acquire unrestricted +command execution or approval authority. + +Remove the generic Assistant resolve/write/verify policy machine rather than +reclassifying protected controls as reads. Registered invocation permissions and +the canonical action lifecycle already enforce the real authority boundaries. +Preserve actual assistant prose and tool results in the transcript. Model-owned +investigation can select further evidence without a tool-count proxy forcing +another turn or manufacturing verification. + +The final-label repeat provides direct evidence for this change. Session +`c601dcaf-2666-4357-95a9-14149ae84e2f` first correctly explained that VM110's +start plan awaited approval. The harness then forced another model turn to +verify a write that had not executed. The model queried the stopped VM and its +history, adding a second conclusion about the unchanged state. Remove the +semantic lifecycle-request gate and first-tool-before-question counters in the +same orchestration review. Tool availability, current evidence and the user's +request should inform model judgment. Keep explicit resource identity, +permission checks, approved action binding and configured spend limits as +mechanical boundaries. + +Required proof includes live missing-runner refusal visible before the final +conclusion, unavailable and healthy no-action conclusions without false failure, +a capability becoming unavailable between lookup and planning, repeated same +plan identity without duplicate action, provider failure after a persisted plan, +unchanged approval/control/tenant restrictions, retained original assistant prose, +and an approved or rejected plan continuing through the shared action history. +This is the next implementation plan, not a claim that those changes exist. + +Qualification must distinguish an expected planning or access refusal from a +defective tool result. The current scorer fails every nonzero failed-tool count, +and its summary-term, resource-name and evidence-count checks do not establish +semantic diagnosis accuracy. Add scenario-owned expected-refusal proof without +waiving unexpected errors. Retain semantic review against independent ground +truth, as the earlier storage false diagnosis demonstrated. Population-level +diagnosis, false-alarm, missed-problem and end-to-end latency rates remain unknown +from adoption and outcome buckets alone. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index f2de43d66..fa2ea42de 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -10201,7 +10201,7 @@ }, { "id": "patrol-assistant-customer-outcome-qualification", - "summary": "The redesign goal remains open. The executable plan and historical source-bound receipts are in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Product ownership, evidence/history/risk corrections and removal of proposal-as-proof and proxy completion rules have landed through PR1928/1929/1934/1935, with enterprise broker refusal handling in PR22. The telemetry baseline of 127 paid installations, 71 Patrol-enabled and 23 with Assistant usage does not establish linked customer effectiveness. Native container filesystem observations now preserve exact bytes, inodes, source, observation time and explicit unavailable errors through collection, ingestion, canonical resources and model tools. The installed native collector and real Gemini storage case q-20260906-232102-5316b904 passed independent semantic review: the model identified the exhausted 8 MiB container tmpfs and ENOSPC, rather than misattributing host array capacity. Recovery was independently observed after fixture filler removal, not after executing the proposal. Fresh healthy, dependency, approved-restart and rejected-restart cases passed their named oracles and cleanup. Tower's original binaries/token were restored, temporary tokens revoked and production agent preserved. Shared event-history WAL isolation, bounded indexed paging and a global resource-history index pass targeted race/query-plan proof. Browser review found expired actions still marked queued, generic starters beside attached Assistant context, and linked action history hidden by outcome-based visibility. Repairs pass final source-bound browser qualification for linked completed/rejected/expired actions and attached/cleared Assistant context at 1440, 900 and 390 pixels. The current slice is awaiting staged-hook verification and landing. Missing-access q-20260907-074158-110e8868 safely refused execution and retained unknown cause, but remains an unqualified journey: proposal capture precedes canonical availability validation, leaving the model unable to incorporate the refusal in its conclusion. The next canonical work must expose current action availability to model decisions and preserve definitive submission truth through the shared journey. Disposable VM110 received an exact Gemini-generated plan and explicit canonical approval. Execution returned HTTP500 and left receipt-pending durable state. The stopped VM, original inactive agent and read-only control were restored, temporary tokens revoked, and the action was closed through operator force-fail with inconclusive evidence. VM execution remains unqualified and its durable dispatch/reconciliation path is the next required root investigation. An additional reproduced orchestration gap replaces retained Assistant prose with an internal verification instruction and treats planned controls as executed writes. The canonical action lifecycle, not generic tool-use state, must own execution/verification truth. Residuals include responsive mount details, canonical incident-memory listing/aliases and failed-read propagation, unsupported filters, typed compatibility ID lookup, legacy direction availability, Docker-host history, backup coverage, broader model qualification and startup latency. Autonomous modes remain unqualified. Independent volunteered Pro environments remain a wider-readiness gate. The explicit Claude subscription refusal was not retried. Do not mark the goal or candidate complete from a scorecard or this one homelab. A later current-runtime attention-summary request timed out after 30 seconds. The captured stacks showed 107 independent full-history walkers. The shared incremental history fold now passes durable replay boundary, retention and concurrent-reader regressions, full alerts/eventlog race suites, eighteen bounded live attention requests and repeated current-runtime browser matrices. Exact staged landing checks remain pending.", + "summary": "The redesign goal remains open. The executable plan and source-bound historical receipts are in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Ownership, evidence/history/risk corrections and removal of proposal-as-proof and proxy diagnosis rules have landed through PR1928/1929/1934/1935 and PR1951, with enterprise broker refusal handling in PR22. The telemetry baseline of 127 paid installations, 71 Patrol-enabled and 23 with Assistant usage does not establish linked customer effectiveness. Real Gemini storage, healthy, dependency, approved-restart and rejected-restart cases passed their named semantic oracles and cleanup. Native filesystem evidence preserves capacity, inodes, source, observation time and unavailable errors. Shared history WAL isolation, indexed paging and incremental replay passed race and bounded functional qualification. Current linked completed/rejected/expired actions and Assistant attached/cleared context passed browser matrices at 1440, 900 and 390 pixels in PR1951. The next slice aligns Proxmox planning and dispatch with unique credential-admitted typed runners and preserves explicit bearer identity under development admin bypass. A first live typed VM110 start and restoration stop both completed with independent Proxmox confirmation. A repeated run exposed a separate shared timestamp contract bug: small positive agent clock skew discarded the completed result and left receipt-pending state. The exact action was closed with inconclusive truth after independent fixture restoration. The uncommitted canonical correction preserves original observer/receiver timestamps, removes Docker and host-update clamping, and bounds Proxmox readback freshness without discarding execution. Final worker shared-evidence and API race proofs passed, followed by a source-bound Darwin enterprise build. The repeated VM110 start and restoration stop both completed with independent Proxmox confirmation and retained terminal native receipts. Pending/completed VM reviews and retained completed/rejected/expired Docker reviews passed final-build Playwright and pixel inspection at 1440, 900 and 390 pixels. Both temporary credentials, services and tunnel were removed, production agent identity was preserved, read-only control was restored and a fresh missing-runner plan refused with HTTP409. This bounded homelab qualification does not qualify the full redesign or autonomous modes. Missing-access q-20260907-074158-110e8868 safely refused execution and retained unknown cause, but proposal capture precedes availability validation, so the model could not incorporate the refusal in its conclusion. The next model-context work must expose canonical current readiness before the decision and preserve submission truth. Assistant still treats a prepared action as an executed write, forces redundant verification, and overwrites retained prose with an internal instruction. Canonical action lifecycle must own execution and verification truth. Remaining scope includes pre-dispatch/reconciliation failure truth, responsive mount details, incident-memory listing/aliases and failed-read propagation, unsupported filters, typed compatibility lookup, legacy direction availability, Docker-host history, backup coverage, broader model qualification and startup latency. Autonomous modes remain unqualified. Independent volunteered Pro environments remain a wider-readiness gate. The explicit Claude subscription refusal was not retried. Do not mark the goal or candidate complete from this one homelab.", "owner": "project-owner", "status": "planned", "recorded_at": "2026-09-05", @@ -10401,21 +10401,7 @@ ] } ], - "work_claims": [ - { - "id": "patrol-filesystem-evidence-coverage-gap-patrol-assistant-customer-outcome-qualification", - "agent_id": "patrol-filesystem-evidence", - "summary": "Finish filesystem evidence, shared expired-action outcome repair and contextual Assistant qualification.", - "target_id": "v6-product-lane-expansion", - "claimed_at": "2026-09-07T07:33:08Z", - "heartbeat_at": "2026-09-07T07:33:08Z", - "expires_at": "2026-09-07T09:33:08Z", - "work_item": { - "kind": "coverage-gap", - "id": "patrol-assistant-customer-outcome-qualification" - } - } - ], + "work_claims": [], "open_decisions": [], "source_of_truth_file": "docs/release-control/v6/internal/SOURCE_OF_TRUTH.md", "resolved_decisions": [ diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 2988204d4..fe0e8fc1c 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1347,10 +1347,10 @@ execution; lifecycle surfaces still consume only the resource payload, action readiness, and action-audit result rather than issuing or approving command-agent grants themselves. Because the agent and Pulse server have independent clocks, the API-owned Docker result projector validates raw -lifecycle/update chronology within its bounded skew window and then binds a -slightly future valid observation to the server receipt boundary for canonical -evidence. Agent lifecycle consumers must not reject that bounded case, widen -the skew window, or clamp stale/excessively future evidence into validity. +lifecycle/update chronology within its bounded skew window, then preserves +both original timestamps in canonical evidence. Agent lifecycle consumers +must not reject bounded skew, widen the freshness window, or rewrite stale +or excessively future evidence into validity. Inside a containerized Unified Agent, the closed start/stop/restart command must inspect and mutate through the Docker / Podman module's already-connected daemon API; it must not depend on a second `docker` or `podman` executable being @@ -1504,7 +1504,14 @@ Proxmox VM and LXC lifecycle affordances follow the same adjacent boundary: lifecycle and fleet surfaces may consume backend-advertised `start`, `shutdown`, `reboot`, and `stop` capabilities and typed `actionReadiness`, but the only execution path is the API-owned action executor that resolves a -connected Proxmox node command agent and records action audit plus verification. +credential-admitted typed action runner for the Proxmox node and records action audit plus verification. +`GetActionRunnerForHostForOrganization` resolves exactly one live runner from +tenant-scoped credential admission. Legacy command sessions, pending or fenced +activation, revoked credentials and ambiguous host matches are not readiness. +The actual dispatch session must satisfy the same closed role/capability +contract. A session replacement cannot inherit a preceding lookup's authority. +Durable operation-receipt support is required before planning. There is no +API executor fallback to raw `qm` or `pct` commands. Lifecycle surfaces must not run `qm` / `pct`, SSH to a node, call Proxmox mutation APIs, or substitute a guest-local agent to perform VM/LXC lifecycle control. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 70b4f5b88..45ab78351 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -2653,10 +2653,10 @@ a new API state machine, queue contract, or verification-accounting field. applies, and the action audit remains the approval and lifecycle source of truth. Docker lifecycle and image-update result evidence must validate raw agent chronology against the bounded freshness/skew window before canonical - normalization. When that valid raw observation is slightly ahead of the - server clock, the API conservatively binds its canonical `ObservedAt` to the - server `ReceivedAt` boundary; stale or excessively future observations stay - inconclusive and must never be made fresh by clamping. + normalization. Canonical evidence preserves the original `ObservedAt` and + server `ReceivedAt` even when the observer clock is slightly ahead. Stale + or excessively future observations stay inconclusive and must never be + made fresh by rewriting timestamps. The version-2 independent Docker observation contract carries a canonical daemon health state as well as lifecycle state. For start and restart, the API may confirm the postcondition only when the container is running and @@ -9405,7 +9405,17 @@ inconclusive result. It must never downgrade execution success or fabricate independence. Proxmox VM and LXC lifecycle execution now consumes that same two-axis truth -contract in production. The node agent remains the executor, while the API +contract in production. Planning and dispatch resolve the same tenant-admitted +typed action runner on the owning Proxmox node. Generic command connectivity +or a telemetry-linked agent ID does not satisfy that authority. Missing typed +transport, runner admission or durable receipts returns the existing +`action_execution_unavailable` response before plan persistence, with +`typed_operation_unavailable`, `action_runner_unavailable` or +`operation_receipt_unsupported` readiness detail. Resource responses expose the +same readiness. The executor has no raw-command fallback. Its typed receipt's +`MutationCompleted` establishes execution completion separately from readback, +and a status-only reboot read remains inconclusive. + The node agent remains the executor, while the API composition root injects the tenant-scoped monitoring client as a direct Proxmox control-plane observer. Fresh, identity-matched control-plane reads use an `agent:*`-distinct trust domain and are digest-bound into `ActionResultV2`; @@ -10728,3 +10738,12 @@ result and unavailable services cannot establish a current count. The old coordinator never received production alert callbacks, so its zero was not a measurement. The legacy listing's broader canonical query and read-error modernization remains open under the customer-outcome qualification gap. + +### Explicit credentials during development qualification + +Development admin bypass must not replace an explicit bearer or API-token +identity with a generic admin. Shared auth context, authentication and admin +gates retain the explicit credential's normal validation and scopes. In +particular, runner activation and self-revocation continue to require the exact +credential-bound host and tenant, including in the managed development runtime. +Invalid explicit credentials cannot fall back to the convenience bypass. diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 267931e28..0e745279f 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -20,6 +20,12 @@ ## Purpose +The shared action evidence disclosure preserves the named observer independently +of the executor. Its observation timestamp uses the neutral label `Observed`, +followed by the separate Pulse receipt time. Independent Proxmox API evidence +must not be labelled as an agent observation. Browser qualification expands +this disclosure in completed action reviews at desktop and narrow widths. + Disk I/O presentation preserves each observed direction independently. Shared formatting renders a missing rate as a dash and measured idle as numeric zero. Partial observations cannot form a complete throughput total for sorting or diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index ab7b9ae9d..71eaeced7 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -9,7 +9,7 @@ "contract_file": "docs/release-control/v6/internal/subsystems/performance-and-scalability.md", "status_file": "docs/release-control/v6/internal/status.json", "registry_file": "docs/release-control/v6/internal/subsystems/registry.json", - "dependency_subsystem_ids": ["api-contracts", "frontend-primitives"] + "dependency_subsystem_ids": ["frontend-primitives"] } ``` @@ -560,7 +560,11 @@ change may globally weaken the Task 03 lifecycle-state idempotency invariant. the provider-authored runtime power state. `useWorkloads.ts` carries both values; selectors must not collapse warning health into `Stopped` or add a second resource lookup to recover it. -4. Keep shared auth gating in `internal/api/router.go` cheap and local: pre-auth quick-setup and recovery routing may short-circuit on loopback/session/token checks, but they must not trigger chart, metrics, or broad persistence fan-out on the protected request hot path. +4. Development bypass checks inspect only the current request's explicit + credential presence and the existing cached development flag. Explicit + tokens continue through canonical authentication and scoped identity, without + introducing a separate auth cache or persistence scan. + Keep shared auth gating in `internal/api/router.go` cheap and local: pre-auth quick-setup and recovery routing may short-circuit on loopback/session/token checks, but they must not trigger chart, metrics, or broad persistence fan-out on the protected request hot path. Agent command authorization is likewise a dispatch-time point lookup and atomic approval consume, not a route-wide scan or request-hot-path fan-out; grant signing and WebSocket writes happen only after that bounded verifier diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index dfebfb8d5..9d01ba198 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -8611,6 +8611,7 @@ "exact_files": [ "internal/ai/tools/filesystem_evidence_test.go", "internal/monitoring/docker_filesystem_evidence_test.go", + "internal/unifiedresources/action_result_v2_test.go", "internal/unifiedresources/availability_link_test.go", "internal/unifiedresources/canonical_id_pins_test.go", "internal/unifiedresources/clone_test.go", diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 3b34d5660..04da70e53 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -98,6 +98,12 @@ shell/exec, unrestricted `read_file`, deploy, and trusted-origin bypasses are forbidden. The legacy combined command channel remains a disclosed full-trust migration boundary only until runner enrollment and live session parity are qualified; it is not safe-profile authority. +Development admin bypass applies only when no explicit API credential is +present. Bearer, X-API-Token and WebSocket token credentials retain normal +validation, tenant identity and scopes. Invalid explicit tokens cannot fall +back to development authority. This preserves the exact bearer identity needed +for runner activation and self-revocation. + The action runner necessarily has a writable host view for its closed mutation set, so `ProtectSystem=strict` is not a valid runner sandbox claim. Its separate credential, no-listener transport, typed admission, target and digest binding, diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 3e9821818..825ce944b 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -920,11 +920,10 @@ host name and dispatches a trusted internal container command after action approval, storage and recovery consumers may observe only the resulting action audit and verification evidence; they must not reinterpret that trusted command path as recovery authority or a storage-owned action transport. Canonical -Docker action evidence may bind a raw, freshness-validated agent observation -that is slightly ahead of the server clock to the server receipt boundary; -storage/recovery consumers must preserve that evidence rather than treating -normal bounded clock skew as recovery failure, and must not apply the same -clamp to observations outside the API-owned freshness window. +Docker action evidence preserves both the raw, freshness-validated observer +clock and the server receipt clock. Storage/recovery consumers must not treat +bounded positive skew as recovery failure or rewrite timestamps to make stale +or excessively future evidence appear valid. Container restart declares rollback unavailable. Its canonical `ActionResultV2.compensation` therefore records support `unavailable` and status `not_available`; neither same-agent readback nor a distinct direct-daemon @@ -6017,3 +6016,13 @@ Malformed, oversized, symlink and non-regular archive paths fail visibly. Missin archives remain distinguishable from a valid archive with no matching window. The original recording times and historical status must never establish current source freshness or active recording. + +### Proxmox guest action runner authority + +The API-owned Proxmox guest executor plans and dispatches only through a live +tenant-admitted typed action runner with durable operation receipts. Generic +node telemetry or a legacy command session cannot authorize a recovery action, +and the executor has no raw-command fallback. Missing runner admission is +reported by canonical action readiness before a plan is persisted. Typed +mutation completion and independently observed guest recovery remain separate +results. A running guest alone cannot prove a reboot. diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 55ae8585f..018250cc8 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -23,6 +23,21 @@ and sort the complete canonical change table while startup and ingestion wait. ## Purpose +Action evidence preserves observer and receiver timestamps as separate clock +facts, normalized to UTC and bound into the canonical digest. The shared +normalizer requires both timestamps but cannot infer their ordering across +machines. Capability-owned freshness, request binding and same-clock mutation +chronology remain prerequisites for claiming verification. Positive clock skew +must not discard a completed execution or rewrite its observed timestamp. +Cross-clock normalization and Proxmox/Docker/host result regressions cover this +boundary, including stale and excessive-skew negative cases. + +The shared action evidence disclosure preserves the named observer independently +of the executor. Its observation timestamp uses the neutral label `Observed`, +followed by the separate Pulse receipt time. Independent Proxmox API evidence +must not be labelled as an agent observation. Browser qualification expands +this disclosure in completed action reviews at desktop and narrow widths. + Canonical Docker resource metadata carries the shared filesystem observation contract unchanged. Adapters, retained resource clones and typed views own their nested usage values. Per-mount filesystem capacity never populates a diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 71ee2dd7c..f98fc3690 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,25 +1,20 @@ { "version": 1, - "base_sha": "560dbf314c4fc3744f52aa4e5a6a204cafe3aa7d", - "verified_at": "2026-09-07T08:40:47Z", + "base_sha": "09ab5c2d0ae6e02fcc5280853782a8142646e40f", + "verified_at": "2026-09-07T10:45:50.469666Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/components/AI/Chat/index.tsx", - "frontend-modern/src/components/AI/FindingsPanel.tsx", - "frontend-modern/src/components/patrol/ApprovalSection.tsx", - "frontend-modern/src/types/resource.ts" + "frontend-modern/src/features/actions/ActionDecisionPacket.tsx" ], "content_sha256": { - "frontend-modern/src/components/AI/Chat/index.tsx": "a31459ed9eeda0e96a192c0b6b471746c1a6da70cd8c68ab75b833ca763071f8", - "frontend-modern/src/components/AI/FindingsPanel.tsx": "92f1fd73740cb71c868c2d7054323d43f93ebc53dbb5cacdc1f828c5c2f2a868", - "frontend-modern/src/components/patrol/ApprovalSection.tsx": "b04e131f5a82d538f37aa9cb3aadaf0f7303266cfc318ff04a42a8f7f6a4f6ba", - "frontend-modern/src/types/resource.ts": "54217a487ce295d433a9e016bf655ab8572a903383ae0516fdd52e5004524ea7" + "frontend-modern/src/features/actions/ActionDecisionPacket.tsx": "ea0301d5ee7bee7b064f8e7fd8db43a54a74bc4488c0a87285d7c52151a5e07b" }, "routes": [ - "/patrol", - "/actions?action=act_185eebf5dfc24652e6332881dbe94f6c", + "/actions?action=act_9ec8d575b777a5f8fc70561376a52381", + "/actions?action=act_3384576233bf55aa3182537ad02d81d4", "/actions?action=act_7687850d214e9c5f7038a436c6cc2ff7", - "/actions?action=act_e5e1f06da3dc42215a97360731d7e36b" + "/actions?action=act_e5e1f06da3dc42215a97360731d7e36b", + "/actions?action=act_185eebf5dfc24652e6332881dbe94f6c" ], "viewports": [ { @@ -36,26 +31,20 @@ } ], "states": [ - "Activity with All findings including resolved records", - "Expired action with needs-attention investigation and retained linked action", - "Completed action with independent verification", - "Rejected action without execution controls", - "Expanded investigation thread with raw filesystem evidence and nested output scroll", - "Assistant empty conversation with attached finding", - "New Assistant conversation with context cleared and ordinary starters restored", - "Existing Assistant conversation loaded from recent sessions", - "Reloaded exact action review" + "VM110 start and stop awaiting exact-plan approval", + "Completed VM110 start and stop with independent Proxmox verification", + "Expanded independent observer evidence with original observation and Pulse receipt labels", + "Expanded durable delivery record with agent-specific observation label", + "Retained completed Docker restart with independent evidence", + "Retained rejected Docker restart without execution controls", + "Retained expired Docker restart with refresh offered and no execution controls" ], "interactions": [ - "Keyboard focus and Enter open the finding review and investigation thread", - "Expand tool results and scroll inside the native filesystem observation output", - "Discuss with Assistant retains the finding and expired action context without unrelated starters", - "Start new Assistant session clears context and restores welcome and recent sessions", - "Load existing session and inspect its transcript", - "Follow exact Actions links, inspect planned state and recorded independent verification", - "Expand policy evidence, evidence details and delivery identifiers with keyboard", - "Reload exact action URLs, close with Escape and the explicit close button", - "Inspect desktop, intermediate and narrow placement, nested scrolling, footer reachability and document overflow" + "Inspect exact target identity, approval controls and planned state before API-authorized execution", + "Open and close policy evidence, evidence details and delivery identifiers using keyboard focus and Enter", + "Scroll nested action review and inspect readable evidence, wrapping, stacking and reachable footer at all widths", + "Close with Escape and explicit close control, reopen exact action link and reload persisted state", + "Inspect final pixels and verify no document horizontal overflow" ], - "notes": "Real bundled development runtime SHA256 1f1f71d2fd77b89f43a980d1e990a4010d2ec467502551555d5ab702f1aa71e6. Playwright scripts and screenshots are in workspace tmp/patrol-filesystem-evidence/{storage-browser,action-browser}. Final pixels were reviewed after transitions settled. Browser proof covers the named changes, not full product readiness. Independent qualification still fails missing-access continuity and VM execution. An existing Assistant verification-policy message leaking into historical transcript is retained as a separate canonical orchestration gap. Mock mode is off. Browser scripts allow only login and selected-provider readiness POSTs. Repeated after the shared incremental alert-history change. Eighteen bounded concurrent attention-summary requests completed and the final goroutine check found no history walkers. This functional check is not a workstation performance benchmark." + "notes": "Final bundled development binary SHA256 c53f2e54e7ea510fc3abfbfbe164e74e363d3b1560aaa36a62df9fb1aaaf2c7d. Scripts, screenshots and raw dialog text are in workspace tmp/patrol-runner-readiness/browser and verify-action.mjs/verify-history.mjs. Browser requests were GET plus login only. Exact VM110 approvals and execution were performed separately through the canonical API, with independent Proxmox and SSH state checks and retained native receipts. Both temporary services, credentials and tunnel were removed and read-only control restored. Deep links have no initiating button for focus return. This receipt qualifies the named action-review change and adjacent rendered outcomes, not full Patrol/Assistant readiness." } diff --git a/frontend-modern/src/features/actions/ActionDecisionPacket.tsx b/frontend-modern/src/features/actions/ActionDecisionPacket.tsx index f1b365dc2..a33e2f26e 100644 --- a/frontend-modern/src/features/actions/ActionDecisionPacket.tsx +++ b/frontend-modern/src/features/actions/ActionDecisionPacket.tsx @@ -303,7 +303,7 @@ export const ActionDecisionPacket: Component<{ Observed by {evidence.observerId} · {evidence.observerTrustDomain}
- Agent observed {new Date(evidence.observedAt).toLocaleString()} · Pulse + Observed {new Date(evidence.observedAt).toLocaleString()} · Pulse received {new Date(evidence.receivedAt).toLocaleString()}
diff --git a/frontend-modern/src/features/actions/__tests__/ActionDecisionPacket.test.tsx b/frontend-modern/src/features/actions/__tests__/ActionDecisionPacket.test.tsx index fc7e0b8dc..b80514897 100644 --- a/frontend-modern/src/features/actions/__tests__/ActionDecisionPacket.test.tsx +++ b/frontend-modern/src/features/actions/__tests__/ActionDecisionPacket.test.tsx @@ -80,7 +80,22 @@ const audit: ActionAuditRecord = { status: 'confirmed', evidenceClass: 'independent', summary: 'A separate observer saw the target state.', - evidence: [], + evidence: [ + { + version: 1, + id: 'observation-1', + observerId: 'docker-daemon:edge', + observerKind: 'docker_daemon', + observerTrustDomain: 'docker-daemon:edge', + executorTrustDomain: 'agent:host-1', + method: 'direct_daemon_readback', + subjectId: 'docker:container:edge', + observedAt: '2026-07-12T00:01:02Z', + receivedAt: '2026-07-12T00:01:00Z', + summary: 'The daemon independently observed a healthy container.', + digest: 'sha256:observation-1', + }, + ], }, compensation: { support: 'unavailable', status: 'not_available' }, }, @@ -106,6 +121,16 @@ describe('ActionDecisionPacket', () => { ).toBeInTheDocument(); expect(screen.getByText('Confirmed by independent observer')).toBeInTheDocument(); expect(screen.getByText('Source: Independent observer')).toBeInTheDocument(); + fireEvent.click(screen.getByText('Evidence details')); + expect( + screen.getByText('Observed by docker-daemon:edge · docker-daemon:edge'), + ).toBeInTheDocument(); + const observedAt = new Date('2026-07-12T00:01:02Z').toLocaleString(); + const receivedAt = new Date('2026-07-12T00:01:00Z').toLocaleString(); + expect( + screen.getByText(`Observed ${observedAt} · Pulse received ${receivedAt}`), + ).toBeInTheDocument(); + expect(screen.queryByText(/Agent observed /)).not.toBeInTheDocument(); }); it.each([ diff --git a/internal/agentexec/server.go b/internal/agentexec/server.go index b0320a814..f838fd046 100644 --- a/internal/agentexec/server.go +++ b/internal/agentexec/server.go @@ -2547,13 +2547,6 @@ func (s *Server) ExecuteProxmoxGuestLifecycle(ctx context.Context, agentID strin if err := ValidateProxmoxGuestLifecyclePayload(&req); err != nil { return nil, err } - ac, ok := s.connectionForContext(ctx, agentID) - if !ok { - return nil, fmt.Errorf("agent %s not connected", agentID) - } - if ac.admission.RuntimeRole != RuntimeRoleActionRunner || ac.admission.ActionCapability != ActionCapabilityTypedV1 { - return nil, fmt.Errorf("Proxmox guest lifecycle requires a typed action-runner session") - } identity := ProxmoxGuestLifecycleOperationIdentity(agentID, req) return dispatchTypedDockerContainerOperation(ctx, s, agentID, req.RequestID, req.Timeout, identity, req.GuestKind+":"+strconv.Itoa(req.VMID), MsgTypeProxmoxGuestLifecycle, req, s.pendingProxmoxGuestLifecycles, "Proxmox guest lifecycle", @@ -2586,6 +2579,11 @@ func dispatchTypedDockerContainerOperation[Res any]( if ac.agent.OperationReceiptVersion != operationreceipt.ProtocolVersion { return nil, fmt.Errorf("agent does not support durable operation receipts") } + // Check the session that will actually carry the request. Checking before + // this lookup would allow a replacement connection to inherit the result. + if msgType == MsgTypeProxmoxGuestLifecycle && !isTypedActionRunner(ac) { + return nil, fmt.Errorf("Proxmox guest lifecycle requires a typed action-runner session") + } respCh := make(chan Res, 1) authorityKey := connectionAuthorityKey(ac) @@ -2910,6 +2908,44 @@ func (s *Server) GetAgentForHost(hostname string) (string, bool) { return s.GetAgentForHostForOrganization(defaultOrganizationID, hostname) } +func isTypedActionRunner(ac *agentConn) bool { + return ac != nil && ac.admission.RuntimeRole == RuntimeRoleActionRunner && + ac.admission.ActionCapability == ActionCapabilityTypedV1 +} + +// GetActionRunnerForHostForOrganization resolves exactly one currently admitted +// typed runner for a canonical host. A collector or legacy command session is +// not action-runner authority. Pending, fenced and revoked sessions are excluded +// by the same admission check used at dispatch. +func (s *Server) GetActionRunnerForHostForOrganization(organizationID, hostname string) (string, bool) { + if s == nil || strings.TrimSpace(hostname) == "" { + return "", false + } + organizationID = normalizeOrganizationID(organizationID) + s.mu.RLock() + ids := make([]string, 0, len(s.agents)) + for _, ac := range s.agents { + if normalizeOrganizationID(ac.admission.OrganizationID) == organizationID && + unifiedresources.HostnamesEquivalent(ac.admission.Hostname, hostname) { + ids = append(ids, ac.agent.AgentID) + } + } + s.mu.RUnlock() + matched := "" + for _, agentID := range ids { + ac, ok := s.connectionForOrganization(organizationID, agentID) + if !ok || !isTypedActionRunner(ac) || + !unifiedresources.HostnamesEquivalent(ac.admission.Hostname, hostname) { + continue + } + if matched != "" { + return "", false + } + matched = agentID + } + return matched, matched != "" +} + // GetAgentForHostForOrganization resolves a hostname only within one tenant. func (s *Server) GetAgentForHostForOrganization(organizationID, hostname string) (string, bool) { if s == nil { diff --git a/internal/agentexec/server_test.go b/internal/agentexec/server_test.go index 02a81b813..d98df2252 100644 --- a/internal/agentexec/server_test.go +++ b/internal/agentexec/server_test.go @@ -61,6 +61,54 @@ func TestActionRefusalReasonCodeContractAndAlreadySatisfiedCleanup(t *testing.T) func allowAllTestTokens(string, string, string) bool { return true } +func TestActionRunnerHostResolutionUsesLiveTenantAdmission(t *testing.T) { + s := NewServerWithAdmissionValidator(func(string, string, string) (AgentAdmission, bool) { + return AgentAdmission{}, false + }, func(a AgentAdmission) bool { return a.TokenID != "revoked" }) + add := func(org, id, host, role, capability, token string) *agentConn { + ac := &agentConn{ + admission: AgentAdmission{OrganizationID: org, AgentID: id, Hostname: host, RuntimeRole: role, ActionCapability: capability, TokenID: token}, + agent: ConnectedAgent{AgentID: id, Hostname: host, RuntimeRole: RuntimeRoleActionRunner, ActionCapability: ActionCapabilityTypedV1, OperationReceiptVersion: 1}, + done: make(chan struct{}), + } + s.agents[agentSessionKey(org, id)] = ac + return ac + } + // Self-reported role/capability must not override credential admission. + add("org-a", "legacy", "delly", RuntimeRoleLegacyFullTrust, "", "legacy") + add("org-a", "unsupported", "delly", RuntimeRoleActionRunner, "unknown", "unsupported") + add("org-a", "revoked", "delly", RuntimeRoleActionRunner, ActionCapabilityTypedV1, "revoked") + add("org-b", "other-tenant", "delly", RuntimeRoleActionRunner, ActionCapabilityTypedV1, "valid") + if _, err := s.ExecuteProxmoxGuestLifecycle(WithOrganizationID(context.Background(), "org-a"), "legacy", boundProxmoxGuestLifecycle(t)); err == nil || !strings.Contains(err.Error(), "requires a typed action-runner session") { + t.Fatalf("legacy session reached typed dispatch: %v", err) + } + if id, ok := s.GetActionRunnerForHostForOrganization("org-a", "delly"); ok || id != "" { + t.Fatalf("non-admitted runner resolved: %q, %v", id, ok) + } + valid := add("org-a", "runner", "delly.home", RuntimeRoleActionRunner, ActionCapabilityTypedV1, "valid") + if id, ok := s.GetActionRunnerForHostForOrganization("org-a", "delly"); !ok || id != "runner" { + t.Fatalf("typed runner not resolved through canonical hostname: %q, %v", id, ok) + } + add("org-a", "ambiguous", "delly.home", RuntimeRoleActionRunner, ActionCapabilityTypedV1, "valid") + if id, ok := s.GetActionRunnerForHostForOrganization("org-a", "delly"); ok || id != "" { + t.Fatalf("ambiguous runner resolved: %q, %v", id, ok) + } + delete(s.agents, agentSessionKey("org-a", "ambiguous")) + s.actionRunnerPromotionFences[agentSessionKey("org-a", "runner")] = &ActionRunnerSessionPromotion{} + if id, ok := s.GetActionRunnerForHostForOrganization("org-a", "delly"); ok || id != "" { + t.Fatalf("fenced runner resolved: %q, %v", id, ok) + } + delete(s.actionRunnerPromotionFences, agentSessionKey("org-a", "runner")) + delete(s.agents, agentSessionKey("org-a", "runner")) + s.pendingActionRunners[agentSessionKey("org-a", "runner")] = valid + if id, ok := s.GetActionRunnerForHostForOrganization("org-a", "delly"); ok || id != "" { + t.Fatalf("pending runner resolved: %q, %v", id, ok) + } + if id, ok := s.GetActionRunnerForHostForOrganization("org-b", "delly"); !ok || id != "other-tenant" { + t.Fatalf("tenant isolation lost: %q, %v", id, ok) + } +} + func TestNewServerRequiresValidateToken(t *testing.T) { defer func() { if recover() == nil { diff --git a/internal/api/admin_bypass_router_test.go b/internal/api/admin_bypass_router_test.go index 3953f39bb..8c6cfc55b 100644 --- a/internal/api/admin_bypass_router_test.go +++ b/internal/api/admin_bypass_router_test.go @@ -4,8 +4,57 @@ import ( "net/http" "net/http/httptest" "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" ) +func TestAdminBypassPreservesExplicitTokenAuthority(t *testing.T) { + t.Setenv("ALLOW_ADMIN_BYPASS", "1") + t.Setenv("PULSE_DEV", "true") + resetAdminBypassState() + t.Cleanup(resetAdminBypassState) + const raw = "scoped-runner-auth-test.12345678" + record := newTokenRecord(t, raw, []string{config.ScopeAgentExec}, nil) + record.OrgID = "default" + cfg := newTestConfigWithTokens(t, record) + router := NewRouter(cfg, nil, nil, nil, nil, "test") + t.Cleanup(router.shutdownBackgroundWorkers) + t.Cleanup(router.ShutdownResourceStores) + t.Cleanup(router.ShutdownRBAC) + router.mux.HandleFunc("/api/qualification-runner-token", RequireAuth(cfg, RequireScope(config.ScopeAgentExec, func(w http.ResponseWriter, req *http.Request) { + actual := getAPITokenRecordFromRequest(req) + if actual == nil || actual.ID != record.ID || isAdminBypassRequest(req.Context()) { + t.Error("explicit runner identity was replaced by development authority") + w.WriteHeader(http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusNoContent) + }))) + router.mux.HandleFunc("/api/qualification-runner-admin", RequireAuth(cfg, RequireScope(config.ScopeSettingsWrite, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }))) + for _, tc := range []struct { + name, header, token, path string + status int + }{ + {"bearer identity", "Authorization", "Bearer " + raw, "/api/qualification-runner-token", http.StatusNoContent}, + {"header identity", "X-API-Token", raw, "/api/qualification-runner-token", http.StatusNoContent}, + {"invalid bearer", "Authorization", "Bearer invalid", "/api/qualification-runner-token", http.StatusUnauthorized}, + {"empty explicit token", "X-API-Token", "", "/api/qualification-runner-token", http.StatusUnauthorized}, + {"scope is retained", "Authorization", "Bearer " + raw, "/api/qualification-runner-admin", http.StatusForbidden}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPatch, tc.path, nil) + req.Header.Set(tc.header, tc.token) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if rec.Code != tc.status { + t.Fatalf("response = %d %s, want %d", rec.Code, rec.Body.String(), tc.status) + } + }) + } +} + func TestAdminBypassDoesNotAllowAdminEndpointsByDefault(t *testing.T) { // Ensure bypass is not enabled t.Setenv("ALLOW_ADMIN_BYPASS", "") diff --git a/internal/api/auth.go b/internal/api/auth.go index bdcdfb58d..3868c31d8 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -670,7 +670,7 @@ func snapshotLocalAuthCredentialsLocked(cfg *config.Config) (string, string) { func checkAuth(cfg *config.Config, w http.ResponseWriter, r *http.Request, writeDefaultFailure bool) bool { // Dev mode bypass for all auth (disabled by default) - if adminBypassEnabled() { + if adminBypassAppliesToRequest(r) { if w != nil { // Set headers for standard admin user w.Header().Set("X-Authenticated-User", "admin") @@ -1019,7 +1019,7 @@ func checkAuth(cfg *config.Config, w http.ResponseWriter, r *http.Request, write func RequireAuth(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // Dev mode bypass for all auth (disabled by default) - if adminBypassEnabled() { + if adminBypassAppliesToRequest(r) { log.Debug(). Str("path", r.URL.Path). Msg("Auth bypass enabled for dev mode") @@ -1051,7 +1051,7 @@ func RequireAuth(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc func RequireAdmin(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // Dev mode bypass for admin endpoints (disabled by default) - if adminBypassEnabled() { + if adminBypassAppliesToRequest(r) { log.Debug(). Str("path", r.URL.Path). Msg("Admin bypass enabled for dev mode") @@ -1305,7 +1305,7 @@ func extractAndStoreAuthContext(cfg *config.Config, mtm *monitoring.MultiTenantM defer config.Mu.RUnlock() // Dev mode bypass - if adminBypassEnabled() { + if adminBypassAppliesToRequest(r) { return attachAdminBypassContext(attachUserContext(r, "admin")) } @@ -1398,6 +1398,15 @@ func getAPITokenRecordFromRequest(r *http.Request) *config.APITokenRecord { clone := record.Clone() return &clone } + +// Explicit credentials retain their identity and scope even in development. +// The convenience bypass must not turn a runner into an unbound admin or let +// an invalid bearer credential fall back to ambient development authority. +func adminBypassAppliesToRequest(r *http.Request) bool { + _, explicitToken := explicitAPITokenFromRequest(r) + return !explicitToken && adminBypassEnabled() +} + func adminBypassEnabled() bool { adminBypassState.once.Do(func() { adminBypassState.enabled, adminBypassState.declined = resolveAdminBypassEnv() diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 157789311..962442741 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -21597,51 +21597,21 @@ func TestContract_DockerLifecycleActionsResolveCommandAgentAndDispatchOneTypedOp } } -func TestContract_DockerActionEvidenceBoundsPositiveAgentClockSkewAtReceipt(t *testing.T) { - source, err := os.ReadFile("docker_container_action_result.go") - if err != nil { - t.Fatalf("read docker_container_action_result.go: %v", err) - } - src := string(source) - for _, snippet := range []string{ - "freshDockerLifecycleObservation(facts.Before.ObservedAt, facts.After.ObservedAt, receivedAt)", - "freshDockerUpdateObservation(facts.After.ObservedAt, receivedAt)", - "func dockerActionEvidenceTimes(observedAt, receivedAt time.Time) (time.Time, time.Time)", - "if observedAt.After(receivedAt) {", - "observedAt = receivedAt", - } { - if !strings.Contains(src, snippet) { - t.Fatalf("Docker action evidence must preserve bounded clock-skew handling snippet %q", snippet) - } - } - if calls := strings.Count(src, "dockerActionEvidenceTimes("); calls != 5 { - t.Fatalf("Docker lifecycle/update agent and independent evidence must all use the receipt-boundary helper; calls = %d, want 5", calls) - } -} - -func TestContract_ProxmoxLifecycleActionsResolveNodeCommandAgentAndVerifyState(t *testing.T) { +func TestContract_ProxmoxLifecycleActionsResolveTypedRunnerAndVerifyState(t *testing.T) { source, err := os.ReadFile("proxmox_guest_action_executor.go") if err != nil { - t.Fatalf("read proxmox_guest_action_executor.go: %v", err) + t.Fatal(err) } src := string(source) - for _, snippet := range []string{ - "func (e proxmoxGuestActionExecutor) connectedProxmoxNodeCommandAgentID(ctx context.Context, resource unified.Resource) (string, error)", - "resource.Proxmox.LinkedAgentID", - "commandAgentForHost(ctx, e.agents, strings.TrimSpace(resource.Proxmox.NodeName))", - "Trusted: true", - "func (e proxmoxGuestActionExecutor) verifyProxmoxGuestState(", - "proxmoxGuestStatusCommand(kind, vmid)", - "agentexec.EvaluateCapabilityPostcondition", - "func (e proxmoxGuestActionExecutor) observeProxmoxGuestPostcondition(", - } { - if !strings.Contains(src, snippet) { - t.Fatalf("proxmox lifecycle executor must pin command-agent/trusted verification snippet %q", snippet) + for _, forbidden := range []string{"ExecuteCommand(", "commandAgentForHost(", "isAgentCommandConnected("} { + if strings.Contains(src, forbidden) { + t.Fatalf("Proxmox lifecycle contains legacy dispatch path %q", forbidden) } } - if strings.Index(src, "if agentID := strings.TrimSpace(resource.Proxmox.LinkedAgentID)") > - strings.Index(src, "commandAgentForHost(ctx, e.agents, strings.TrimSpace(resource.Proxmox.NodeName))") { - t.Fatal("proxmox lifecycle executor must try the linked Proxmox node agent before falling back to node hostname resolution") + for _, required := range []string{"GetActionRunnerForHostForOrganization", "ExecuteProxmoxGuestLifecycle", "ValidateProxmoxGuestLifecycleResultForRequest", "observeProxmoxGuestPostcondition"} { + if !strings.Contains(src, required) { + t.Fatalf("Proxmox lifecycle missing typed dispatch contract %q", required) + } } router, err := os.ReadFile("router.go") diff --git a/internal/api/docker_container_action_result.go b/internal/api/docker_container_action_result.go index 7d67f9e2a..b5837501f 100644 --- a/internal/api/docker_container_action_result.go +++ b/internal/api/docker_container_action_result.go @@ -25,11 +25,10 @@ func dockerContainerExecutionResult(resourceID, agentID string, req agentexec.Do verification.ReasonCode = "stale_agent_readback" verification.Summary = "The agent readback was stale, skewed, or had invalid mutation chronology." } else { - evidenceObservedAt, evidenceReceivedAt := dockerActionEvidenceTimes(facts.After.ObservedAt, receivedAt) evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ Version: unified.ActionEvidenceVersion, ID: req.Operation + "-agent-readback", ObserverID: agentID, ObserverKind: "unified_agent", ObserverTrustDomain: "agent:" + agentID, ExecutorTrustDomain: "agent:" + agentID, - Method: "typed_container_read_after_write", SubjectID: resourceID, ObservedAt: evidenceObservedAt, ReceivedAt: evidenceReceivedAt, Summary: summary, + Method: "typed_container_read_after_write", SubjectID: resourceID, ObservedAt: facts.After.ObservedAt.UTC(), ReceivedAt: receivedAt.UTC(), Summary: summary, }) if err != nil { return nil, fmt.Errorf("normalize docker lifecycle evidence: %w", err) @@ -47,7 +46,6 @@ func dockerContainerExecutionResult(resourceID, agentID string, req agentexec.Do } } if independent != nil && freshDockerLifecycleObservation(facts.Before.ObservedAt, independent.Snapshot.ObservedAt, independent.ReceivedAt) && strings.TrimSpace(independent.TrustDomain) != "" && strings.TrimSpace(independent.TrustDomain) != "agent:"+agentID && strings.EqualFold(independent.Snapshot.ContainerID, facts.ContainerID) { - evidenceObservedAt, evidenceReceivedAt := dockerActionEvidenceTimes(independent.Snapshot.ObservedAt, independent.ReceivedAt) observationDigest, err := operationreceipt.DigestCanonicalJSON(struct { ActionID string `json:"action_id"` SubjectID string `json:"subject_id"` @@ -61,7 +59,7 @@ func dockerContainerExecutionResult(resourceID, agentID string, req agentexec.Do evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ Version: unified.ActionEvidenceVersion, ID: req.ActionID + "-direct-daemon-observation", ObserverID: independent.ObserverID, ObserverKind: "docker_daemon_observer", ObserverTrustDomain: independent.TrustDomain, ExecutorTrustDomain: "agent:" + agentID, - Method: independent.Method, SubjectID: resourceID, ObservedAt: evidenceObservedAt, ReceivedAt: evidenceReceivedAt, + Method: independent.Method, SubjectID: resourceID, ObservedAt: independent.Snapshot.ObservedAt.UTC(), ReceivedAt: independent.ReceivedAt.UTC(), Summary: independentSummary, Refs: []unified.ActionEvidenceRef{{ID: req.ActionID, Kind: "docker_before_after", Digest: observationDigest}}, }) if err != nil { @@ -104,11 +102,10 @@ func dockerContainerUpdateExecutionResult(resourceID, agentID string, facts agen verification.ReasonCode = "stale_agent_readback" verification.Summary = "The agent readback was stale or skewed." } else { - evidenceObservedAt, evidenceReceivedAt := dockerActionEvidenceTimes(facts.After.ObservedAt, receivedAt) evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ Version: unified.ActionEvidenceVersion, ID: facts.Operation + "-agent-readback", ObserverID: agentID, ObserverKind: "unified_agent", ObserverTrustDomain: "agent:" + agentID, ExecutorTrustDomain: "agent:" + agentID, - Method: "typed_container_read_after_write", SubjectID: resourceID, ObservedAt: evidenceObservedAt, ReceivedAt: evidenceReceivedAt, Summary: summary, + Method: "typed_container_read_after_write", SubjectID: resourceID, ObservedAt: facts.After.ObservedAt.UTC(), ReceivedAt: receivedAt.UTC(), Summary: summary, }) if err != nil { return nil, fmt.Errorf("normalize docker update evidence: %w", err) @@ -131,7 +128,6 @@ func dockerContainerUpdateExecutionResult(resourceID, agentID string, facts agen } } if independent != nil && facts.NewContainerID != "" && freshDockerUpdateObservation(independent.Snapshot.ObservedAt, independent.ReceivedAt) && strings.TrimSpace(independent.TrustDomain) != "" && strings.TrimSpace(independent.TrustDomain) != "agent:"+agentID { - evidenceObservedAt, evidenceReceivedAt := dockerActionEvidenceTimes(independent.Snapshot.ObservedAt, independent.ReceivedAt) observationDigest, err := operationreceipt.DigestCanonicalJSON(struct { ActionID string `json:"action_id"` SubjectID string `json:"subject_id"` @@ -145,7 +141,7 @@ func dockerContainerUpdateExecutionResult(resourceID, agentID string, facts agen evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ Version: unified.ActionEvidenceVersion, ID: facts.ActionID + "-direct-daemon-observation", ObserverID: independent.ObserverID, ObserverKind: "docker_daemon_observer", ObserverTrustDomain: independent.TrustDomain, ExecutorTrustDomain: "agent:" + agentID, - Method: independent.Method, SubjectID: resourceID, ObservedAt: evidenceObservedAt, ReceivedAt: evidenceReceivedAt, + Method: independent.Method, SubjectID: resourceID, ObservedAt: independent.Snapshot.ObservedAt.UTC(), ReceivedAt: independent.ReceivedAt.UTC(), Summary: independentSummary, Refs: []unified.ActionEvidenceRef{{ID: facts.ActionID, Kind: "docker_update_replacement", Digest: observationDigest}}, }) if err != nil { @@ -258,20 +254,6 @@ func freshDockerUpdateObservation(observedAt, receivedAt time.Time) bool { return !observedAt.Before(receivedAt.UTC().Add(-30*time.Minute)) && !observedAt.After(receivedAt.UTC().Add(5*time.Minute)) } -// dockerActionEvidenceTimes preserves the server receipt boundary required by -// canonical evidence while tolerating bounded positive clock skew already -// admitted by the Docker freshness checks. The raw timestamps are validated -// before this helper is called, so an excessively future or stale observation -// still fails closed instead of being made fresh by clamping. -func dockerActionEvidenceTimes(observedAt, receivedAt time.Time) (time.Time, time.Time) { - observedAt = observedAt.UTC() - receivedAt = receivedAt.UTC() - if observedAt.After(receivedAt) { - observedAt = receivedAt - } - return observedAt, receivedAt -} - // dockerUpdateFactsMatch confirms the observed container is the replacement // the agent claims to have created. Independent verification additionally // compares its state with the agent readback, preserving stopped updates. diff --git a/internal/api/docker_container_action_result_test.go b/internal/api/docker_container_action_result_test.go index b0365279e..5292fe4d8 100644 --- a/internal/api/docker_container_action_result_test.go +++ b/internal/api/docker_container_action_result_test.go @@ -71,7 +71,7 @@ func TestDockerContainerExecutionResultStaleReadbackIsInconclusive(t *testing.T) } } -func TestDockerContainerExecutionResultClampsBoundedPositiveAgentClockSkew(t *testing.T) { +func TestDockerContainerExecutionResultPreservesBoundedPositiveAgentClockSkew(t *testing.T) { receivedAt := time.Now().UTC() facts := dockerResultFacts(receivedAt.Add(2*time.Second), true, true, true, true) result, err := dockerContainerExecutionResult("app-container:fixture", "agent-1", agentexec.DockerContainerLifecyclePayload{Operation: agentexec.DockerContainerOperationRestart}, facts, nil, receivedAt) @@ -82,8 +82,8 @@ func TestDockerContainerExecutionResultClampsBoundedPositiveAgentClockSkew(t *te if verification.Status != unified.ActionVerificationConfirmed || len(verification.Evidence) != 1 { t.Fatalf("verification = %#v", verification) } - if !verification.Evidence[0].ObservedAt.Equal(receivedAt) || !verification.Evidence[0].ReceivedAt.Equal(receivedAt) { - t.Fatalf("evidence timestamps = observed %s received %s, want receipt boundary %s", verification.Evidence[0].ObservedAt, verification.Evidence[0].ReceivedAt, receivedAt) + if !verification.Evidence[0].ObservedAt.Equal(facts.After.ObservedAt) || !verification.Evidence[0].ReceivedAt.Equal(receivedAt) { + t.Fatalf("evidence timestamps = observed %s received %s, want original observation and receipt boundary %s", verification.Evidence[0].ObservedAt, verification.Evidence[0].ReceivedAt, receivedAt) } } @@ -100,7 +100,7 @@ func TestDockerContainerExecutionResultRejectsExcessivePositiveAgentClockSkew(t } } -func TestDockerContainerUpdateExecutionResultClampsBoundedPositiveAgentClockSkew(t *testing.T) { +func TestDockerContainerUpdateExecutionResultPreservesBoundedPositiveAgentClockSkew(t *testing.T) { receivedAt := time.Now().UTC() observedAt := receivedAt.Add(2 * time.Second) facts := agentexec.DockerContainerUpdateResultPayload{ @@ -116,8 +116,8 @@ func TestDockerContainerUpdateExecutionResultClampsBoundedPositiveAgentClockSkew if verification.Status != unified.ActionVerificationConfirmed || len(verification.Evidence) != 1 { t.Fatalf("verification = %#v", verification) } - if !verification.Evidence[0].ObservedAt.Equal(receivedAt) || !verification.Evidence[0].ReceivedAt.Equal(receivedAt) { - t.Fatalf("evidence timestamps = observed %s received %s, want receipt boundary %s", verification.Evidence[0].ObservedAt, verification.Evidence[0].ReceivedAt, receivedAt) + if !verification.Evidence[0].ObservedAt.Equal(observedAt) || !verification.Evidence[0].ReceivedAt.Equal(receivedAt) { + t.Fatalf("evidence timestamps = observed %s received %s, want original observation and receipt boundary %s", verification.Evidence[0].ObservedAt, verification.Evidence[0].ReceivedAt, receivedAt) } } diff --git a/internal/api/host_apt_action_result.go b/internal/api/host_apt_action_result.go index cd5097581..42846d9b8 100644 --- a/internal/api/host_apt_action_result.go +++ b/internal/api/host_apt_action_result.go @@ -39,19 +39,10 @@ func hostAPTExecutionResult(resourceID, agentID, operation, output, refusalReaso verificationTruth.ReasonCode = "stale_agent_readback" verificationTruth.Summary = "The agent readback was stale, skewed, or had invalid mutation chronology." } else { - evidenceObservedAt := afterObservedAt.UTC() - evidenceReceivedAt := receivedAt.UTC() - // Validation permits the agent clock to be slightly ahead of the - // server. Canonical evidence cannot claim an observation after its - // receipt, so conservatively bind bounded positive skew to the - // server receipt boundary. - if evidenceObservedAt.After(evidenceReceivedAt) { - evidenceObservedAt = evidenceReceivedAt - } evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ Version: unified.ActionEvidenceVersion, ID: operation + "-agent-readback", ObserverID: agentID, ObserverKind: "unified_agent", ObserverTrustDomain: "agent:" + agentID, ExecutorTrustDomain: "agent:" + agentID, - Method: "typed_read_after_write", SubjectID: resourceID, ObservedAt: evidenceObservedAt, ReceivedAt: evidenceReceivedAt, + Method: "typed_read_after_write", SubjectID: resourceID, ObservedAt: afterObservedAt.UTC(), ReceivedAt: receivedAt.UTC(), Summary: output, }) if err == nil { diff --git a/internal/api/host_apt_action_result_test.go b/internal/api/host_apt_action_result_test.go index 212af9971..55552c079 100644 --- a/internal/api/host_apt_action_result_test.go +++ b/internal/api/host_apt_action_result_test.go @@ -52,7 +52,7 @@ func TestHostAPTActionResultFutureVerifiedClaimFailsClosed(t *testing.T) { } } -func TestHostAPTActionResultBoundsPermittedPositiveClockSkewToReceipt(t *testing.T) { +func TestHostAPTActionResultPreservesPermittedPositiveClockSkew(t *testing.T) { now := time.Date(2026, 7, 12, 9, 0, 0, 0, time.UTC) result, err := hostAPTExecutionResult("agent:host-1", "host-1", agentexec.HostStorageCleanupOperationPackageCache, "cleanup complete", "", true, true, agentexec.HostStorageCleanupVerificationVerified, true, false, false, false, false, now.Add(-time.Second), now.Add(time.Second), now, now) if err != nil { @@ -62,8 +62,8 @@ func TestHostAPTActionResultBoundsPermittedPositiveClockSkewToReceipt(t *testing if truth.Status != unified.ActionVerificationConfirmed || truth.EvidenceClass != unified.ActionEvidenceAgentAttested || len(truth.Evidence) != 1 { t.Fatalf("bounded clock skew lost verified readback: %#v", truth) } - if !truth.Evidence[0].ObservedAt.Equal(now) || !truth.Evidence[0].ReceivedAt.Equal(now) { - t.Fatalf("bounded clock skew was not conservatively normalized: %#v", truth.Evidence[0]) + if !truth.Evidence[0].ObservedAt.Equal(now.Add(time.Second)) || !truth.Evidence[0].ReceivedAt.Equal(now) { + t.Fatalf("original observation and receipt times were not preserved: %#v", truth.Evidence[0]) } } diff --git a/internal/api/proxmox_guest_action_executor.go b/internal/api/proxmox_guest_action_executor.go index c778076bf..afea7f6ff 100644 --- a/internal/api/proxmox_guest_action_executor.go +++ b/internal/api/proxmox_guest_action_executor.go @@ -54,6 +54,7 @@ type proxmoxGuestPostconditionObserver interface { } type proxmoxGuestLifecycleAgentCommander interface { + GetActionRunnerForHostForOrganization(organizationID, hostname string) (string, bool) ExecuteProxmoxGuestLifecycle(context.Context, string, agentexec.ProxmoxGuestLifecyclePayload) (*agentexec.ProxmoxGuestLifecycleResultPayload, error) } @@ -73,7 +74,7 @@ func (e proxmoxGuestActionExecutor) BindActionDispatch(ctx context.Context, reco if err != nil { return unified.ActionDispatchAttempt{}, err } - agentID, err := e.connectedProxmoxNodeCommandAgentID(ctx, resource) + agentID, err := e.connectedProxmoxNodeActionRunnerID(ctx, resource) if err != nil { return unified.ActionDispatchAttempt{}, err } @@ -111,7 +112,7 @@ func (e proxmoxGuestActionExecutor) ExecuteAction(ctx context.Context, record un return nil, err } vmid := resource.Proxmox.VMID - agentID, err := e.connectedProxmoxNodeCommandAgentID(ctx, resource) + agentID, err := e.connectedProxmoxNodeActionRunnerID(ctx, resource) if err != nil { return nil, err } @@ -123,65 +124,47 @@ func (e proxmoxGuestActionExecutor) ExecuteAction(ctx context.Context, record un } actionStartedAt := time.Now().UTC() - if typedAgents, ok := e.agents.(proxmoxGuestLifecycleAgentCommander); ok { - request, err := proxmoxGuestLifecycleRequest(attempt.ID, record.ID, string(kind), operation, resource) - if err != nil { - return nil, err - } - if attempt.OperationKind != "" && agentexec.ProxmoxGuestLifecycleOperationIdentity(agentID, request) != (operationreceipt.Identity{AttemptID: attempt.ID, ActionID: attempt.ActionID, OperationKind: attempt.OperationKind, OperationVersion: attempt.OperationVersion, RequestDigest: attempt.RequestDigest, AgentID: attempt.AgentID}) { - return nil, fmt.Errorf("Proxmox guest lifecycle dispatch binding drift") - } - result, err := typedAgents.ExecuteProxmoxGuestLifecycle(agentCommandContext(ctx), agentID, request) - if err != nil { - return nil, err - } - if result == nil { - return nil, fmt.Errorf("typed Proxmox guest lifecycle returned no result") - } - if err := agentexec.ValidateProxmoxGuestLifecycleResultForRequest(request, *result); err != nil { - return nil, fmt.Errorf("invalid typed Proxmox guest lifecycle result: %w", err) - } - succeeded := result.ExecutionPhase == agentexec.ProxmoxGuestPhaseComplete && result.MutationCompleted && result.Error == "" - exitCode := 0 - if !succeeded { - exitCode = 1 - } - output := "" - if result.ReadbackRan { - output = "status: " + result.After.Status - } - agentVerification := &unified.ActionVerificationResult{ - Ran: result.ReadbackRan, Command: proxmoxGuestStatusCommand(kind, vmid), Output: output, - Success: succeeded, RanAt: result.After.ObservedAt, - } - independentAfter, independentEvaluation := e.observeProxmoxGuestPostcondition(ctx, record.Request.ResourceID, resource, kind, operation, independentBefore, actionStartedAt) - return proxmoxGuestExecutionResult(record.ID, record.Request.ResourceID, agentID, kind, operation, exitCode, output, result.Error, agentVerification, independentBefore, independentAfter, independentEvaluation, actionStartedAt) - } - - // Legacy full-trust sessions retain the historical command boundary during - // migration. Typed action-runner sessions can never enter this fallback: - // the server rejects execute_command for that runtime role. - command := proxmoxGuestLifecycleCommand(kind, operation, vmid) - result, err := e.agents.ExecuteCommand(agentCommandContext(ctx), agentID, agentexec.ExecuteCommandPayload{ - RequestID: attempt.ID, - Command: command, - ApprovalID: record.ID, - TargetType: "agent", - Timeout: proxmoxGuestLifecycleTimeout(operation), - Trusted: true, - }) + typedAgents := e.agents.(proxmoxGuestLifecycleAgentCommander) + request, err := proxmoxGuestLifecycleRequest(attempt.ID, record.ID, string(kind), operation, resource) if err != nil { return nil, err } - - output := redactActionOutput(commandOutput(result)) - if result.ExitCode != 0 { - return proxmoxGuestExecutionResult(record.ID, record.Request.ResourceID, agentID, kind, operation, result.ExitCode, output, result.Error, nil, independentBefore, nil, agentexec.PostconditionEvaluation{}, actionStartedAt) + if attempt.OperationKind != "" && agentexec.ProxmoxGuestLifecycleOperationIdentity(agentID, request) != (operationreceipt.Identity{AttemptID: attempt.ID, ActionID: attempt.ActionID, OperationKind: attempt.OperationKind, OperationVersion: attempt.OperationVersion, RequestDigest: attempt.RequestDigest, AgentID: attempt.AgentID}) { + return nil, fmt.Errorf("Proxmox guest lifecycle dispatch binding drift") + } + result, err := typedAgents.ExecuteProxmoxGuestLifecycle(agentCommandContext(ctx), agentID, request) + agentReceivedAt := time.Now().UTC() + if err != nil { + return nil, err + } + if result == nil { + return nil, fmt.Errorf("typed Proxmox guest lifecycle returned no result") + } + if err := agentexec.ValidateProxmoxGuestLifecycleResultForRequest(request, *result); err != nil { + return nil, fmt.Errorf("invalid typed Proxmox guest lifecycle result: %w", err) + } + succeeded := result.MutationCompleted + exitCode := 0 + if !succeeded { + exitCode = 1 + } + output := "" + if result.ReadbackRan { + output = "status: " + result.After.Status + } + agentVerification := &unified.ActionVerificationResult{Ran: false} + // Completing the mutation and observing its postcondition are separate + // facts. A status-only read cannot establish that a reboot occurred. + if succeeded && result.ReadbackRan && operation != "reboot" { + evaluation, _ := agentexec.EvaluateCapabilityPostcondition(proxmoxPostconditionCapability(kind, operation), nil, + proxmoxGuestPostconditionValues(kind, result.After.Status, 0), actionStartedAt) + agentVerification = &unified.ActionVerificationResult{ + Ran: evaluation.Conclusive, Command: proxmoxGuestStatusCommand(kind, vmid), Output: output, + Success: evaluation.Matched, RanAt: result.After.ObservedAt, Note: result.Error, + } } - - agentVerification := e.verifyProxmoxGuestState(ctx, agentID, record.ID, kind, vmid, operation, actionStartedAt) independentAfter, independentEvaluation := e.observeProxmoxGuestPostcondition(ctx, record.Request.ResourceID, resource, kind, operation, independentBefore, actionStartedAt) - return proxmoxGuestExecutionResult(record.ID, record.Request.ResourceID, agentID, kind, operation, result.ExitCode, output, result.Error, agentVerification, independentBefore, independentAfter, independentEvaluation, actionStartedAt) + return proxmoxGuestExecutionResult(record.ID, record.Request.ResourceID, agentID, kind, operation, exitCode, output, result.Error, agentVerification, independentBefore, independentAfter, independentEvaluation, actionStartedAt, agentReceivedAt) } func proxmoxGuestLifecycleRequest(attemptID, actionID, kind, operation string, resource unified.Resource) (agentexec.ProxmoxGuestLifecyclePayload, error) { @@ -222,8 +205,15 @@ func (e proxmoxGuestActionExecutor) CheckActionAvailable(ctx context.Context, re if _, _, err := e.executableProxmoxGuestResource(resource, operation); err != nil { return unavailableProxmoxActionReadiness(operation, proxmoxActionUnavailableReasonCode(err), proxmoxActionUnavailableReason(err)) } - if _, err := e.connectedProxmoxNodeCommandAgentID(ctx, resource); err != nil { - return unavailableProxmoxActionReadiness(operation, "command_agent_disconnected", "Proxmox node command agent is not connected.") + if _, ok := e.agents.(proxmoxGuestLifecycleAgentCommander); !ok { + return unavailableProxmoxActionReadiness(operation, "typed_operation_unavailable", "Typed Proxmox lifecycle execution is not available.") + } + agentID, err := e.connectedProxmoxNodeActionRunnerID(ctx, resource) + if err != nil { + return unavailableProxmoxActionReadiness(operation, "action_runner_unavailable", "Connect a typed action runner on this Proxmox node before planning a guest action.") + } + if liveAgentOperationReceiptVersion(ctx, e.agents, agentID) != operationreceipt.ProtocolVersion { + return unavailableProxmoxActionReadiness(operation, "operation_receipt_unsupported", "The Proxmox action runner cannot retain durable action receipts. Update the runner or restore its writable state directory, then retry.") } return readiness } @@ -276,23 +266,18 @@ func (e proxmoxGuestActionExecutor) executableProxmoxGuestResource(resource unif return resource, kind, nil } -func (e proxmoxGuestActionExecutor) connectedProxmoxNodeCommandAgentID(ctx context.Context, resource unified.Resource) (string, error) { - if e.agents == nil { - return "", fmt.Errorf("proxmox node command agent is not connected") - } +func (e proxmoxGuestActionExecutor) connectedProxmoxNodeActionRunnerID(ctx context.Context, resource unified.Resource) (string, error) { if resource.Proxmox == nil { return "", fmt.Errorf("proxmox resource metadata missing") } - if agentID := strings.TrimSpace(resource.Proxmox.LinkedAgentID); agentID != "" && isAgentCommandConnected(ctx, e.agents, agentID) { + runners, ok := e.agents.(proxmoxGuestLifecycleAgentCommander) + if !ok { + return "", fmt.Errorf("typed Proxmox lifecycle execution is unavailable") + } + if agentID, connected := runners.GetActionRunnerForHostForOrganization(GetOrgID(ctx), resource.Proxmox.NodeName); connected && strings.TrimSpace(agentID) != "" { return agentID, nil } - if agentID, ok := commandAgentForHost(ctx, e.agents, strings.TrimSpace(resource.Proxmox.NodeName)); ok { - agentID = strings.TrimSpace(agentID) - if agentID != "" && isAgentCommandConnected(ctx, e.agents, agentID) { - return agentID, nil - } - } - return "", fmt.Errorf("proxmox node command agent is not connected") + return "", fmt.Errorf("a typed action runner is not connected on the Proxmox node") } func proxmoxGuestKindAndHandler(resource unified.Resource) (proxmoxGuestKind, string, error) { @@ -389,14 +374,6 @@ func isProxmoxGuestLifecycleOperation(operation string) bool { } } -func proxmoxGuestLifecycleCommand(kind proxmoxGuestKind, operation string, vmid int) string { - tool := "qm" - if kind == proxmoxGuestCT { - tool = "pct" - } - return strings.Join([]string{tool, strings.TrimSpace(operation), strconv.Itoa(vmid)}, " ") -} - func proxmoxGuestStatusCommand(kind proxmoxGuestKind, vmid int) string { tool := "qm" if kind == proxmoxGuestCT { @@ -414,66 +391,6 @@ func proxmoxGuestLifecycleTimeout(operation string) int { } } -func (e proxmoxGuestActionExecutor) verifyProxmoxGuestState(ctx context.Context, agentID, actionID string, kind proxmoxGuestKind, vmid int, operation string, actionStartedAt time.Time) *unified.ActionVerificationResult { - capability := proxmoxPostconditionCapability(kind, operation) - if _, ok := agentexec.LookupCapabilityPostcondition(capability); !ok { - return &unified.ActionVerificationResult{Ran: false, Note: "No registered postcondition is available for this Proxmox action."} - } - if strings.EqualFold(strings.TrimSpace(operation), "reboot") { - return &unified.ActionVerificationResult{Ran: false, Note: "The executing agent's status-only read cannot prove that the guest restarted."} - } - command := proxmoxGuestStatusCommand(kind, vmid) - - var lastOutput string - var lastEvaluation agentexec.PostconditionEvaluation - for attempt := 0; attempt < 5; attempt++ { - if attempt > 0 { - timer := time.NewTimer(1 * time.Second) - select { - case <-ctx.Done(): - timer.Stop() - return &unified.ActionVerificationResult{Ran: false} - case <-timer.C: - } - } - - result, err := e.agents.ExecuteCommand(agentCommandContext(ctx), agentID, agentexec.ExecuteCommandPayload{ - RequestID: fmt.Sprintf("%s-verify-%d", actionID, attempt+1), - Command: command, - ApprovalID: actionID, - TargetType: "agent", - Timeout: 30, - Trusted: true, - }) - if err != nil { - return &unified.ActionVerificationResult{Ran: false} - } - lastOutput = redactActionOutput(commandOutput(result)) - if result.ExitCode != 0 { - continue - } - lastEvaluation, _ = agentexec.EvaluateCapabilityPostcondition(capability, nil, proxmoxGuestPostconditionValues(kind, parseProxmoxGuestStatus(lastOutput), 0), actionStartedAt) - if lastEvaluation.Conclusive && lastEvaluation.Matched { - return &unified.ActionVerificationResult{ - Ran: true, - Command: command, - Output: lastOutput, - Success: true, - RanAt: time.Now().UTC(), - } - } - } - - return &unified.ActionVerificationResult{ - Ran: true, - Command: command, - Output: lastOutput, - Success: false, - RanAt: time.Now().UTC(), - Note: firstNonEmpty(lastEvaluation.ReasonCode, "postcondition was not confirmed"), - } -} - func (e proxmoxGuestActionExecutor) observeProxmoxGuestPostcondition(ctx context.Context, resourceID string, resource unified.Resource, kind proxmoxGuestKind, operation string, before *proxmoxGuestPostconditionObservation, actionStartedAt time.Time) (*proxmoxGuestPostconditionObservation, agentexec.PostconditionEvaluation) { if e.observer == nil || resource.Proxmox == nil { return nil, agentexec.PostconditionEvaluation{} diff --git a/internal/api/proxmox_guest_action_executor_test.go b/internal/api/proxmox_guest_action_executor_test.go index 93feccb51..b8bc071de 100644 --- a/internal/api/proxmox_guest_action_executor_test.go +++ b/internal/api/proxmox_guest_action_executor_test.go @@ -16,6 +16,128 @@ import ( unified "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) +type fakeProxmoxActionAgentCommander struct { + calls []agentexec.ProxmoxGuestLifecyclePayload + callAgents []string + connected map[string]bool + agentByHost map[string]string + afterStatus string + mutateResult func(*agentexec.ProxmoxGuestLifecycleResultPayload) + receiptVersion *int + organizationID string +} + +func (f *fakeProxmoxActionAgentCommander) ExecuteCommand(context.Context, string, agentexec.ExecuteCommandPayload) (*agentexec.CommandResultPayload, error) { + panic("Proxmox actions must not dispatch generic commands") +} +func (f *fakeProxmoxActionAgentCommander) GetAgentForHost(string) (string, bool) { + panic("Proxmox actions must not discover generic command sessions") +} +func (f *fakeProxmoxActionAgentCommander) IsAgentConnected(string) bool { + panic("generic connectivity is not typed runner authority") +} +func (f *fakeProxmoxActionAgentCommander) GetActionRunnerForHostForOrganization(org, hostname string) (string, bool) { + f.organizationID = org + id := "node-agent-1" + if f.agentByHost != nil { + id = f.agentByHost[hostname] + } + return id, id != "" && (f.connected == nil || f.connected[id]) +} +func (f *fakeProxmoxActionAgentCommander) AgentOperationReceiptVersion(string) int { + if f.receiptVersion != nil { + return *f.receiptVersion + } + return 1 +} +func (f *fakeProxmoxActionAgentCommander) ExecuteProxmoxGuestLifecycle(_ context.Context, agentID string, req agentexec.ProxmoxGuestLifecyclePayload) (*agentexec.ProxmoxGuestLifecycleResultPayload, error) { + f.calls = append(f.calls, req) + f.callAgents = append(f.callAgents, agentID) + status := "running" + if req.Operation == "stop" || req.Operation == "shutdown" { + status = "stopped" + } + if f.afterStatus != "" { + status = f.afterStatus + } + now := time.Now().UTC() + result := &agentexec.ProxmoxGuestLifecycleResultPayload{ + RequestID: req.RequestID, ActionID: req.ActionID, Operation: req.Operation, + OperationVersion: req.OperationVersion, RequestDigest: req.RequestDigest, + GuestKind: req.GuestKind, VMID: req.VMID, ExecutionPhase: agentexec.ProxmoxGuestPhaseComplete, + MutationStarted: true, MutationCompleted: true, ReadbackRan: true, + Before: agentexec.ProxmoxGuestLifecycleSnapshot{Status: req.ExpectedStatus, ObservedAt: now}, + After: agentexec.ProxmoxGuestLifecycleSnapshot{Status: status, ObservedAt: now}, + } + if f.mutateResult != nil { + f.mutateResult(result) + } + return result, nil +} + +func TestProxmoxGuestPlanningRejectsLegacySessionAndMissingReceipts(t *testing.T) { + noReceipts := 0 + for _, tc := range []struct { + name string + agents actionAgentCommander + code string + }{ + {"legacy connected", &fakeDockerActionAgentCommander{}, "typed_operation_unavailable"}, + {"no admitted runner", &fakeProxmoxActionAgentCommander{connected: map[string]bool{}}, "action_runner_unavailable"}, + {"receipts unavailable", &fakeProxmoxActionAgentCommander{receiptVersion: &noReceipts}, "operation_receipt_unsupported"}, + } { + t.Run(tc.name, func(t *testing.T) { + now := time.Now().UTC() + h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) + h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now)}}) + h.SetActionExecutor(newRoutedActionExecutor(h, newProxmoxGuestActionExecutor(h, tc.agents, nil))) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/actions/plan", bytes.NewBufferString(`{"requestId":"runner-check","resourceId":"vm:160","capabilityName":"shutdown","reason":"operator requested shutdown","requestedBy":"operator"}`)) + h.HandlePlanAction(rec, actionHandlerTestRequest(req, "")) + if rec.Code != http.StatusConflict || !strings.Contains(rec.Body.String(), `"reasonCode":"`+tc.code+`"`) { + t.Fatalf("plan response = %d %s", rec.Code, rec.Body.String()) + } + store, err := h.getStore("default") + if err != nil { + t.Fatal(err) + } + audits, err := store.GetActionAudits("vm:160", time.Time{}, 10) + if err != nil || len(audits) != 0 { + t.Fatalf("refused plan wrote audit: %#v, %v", audits, err) + } + }) + } +} + +func TestProxmoxTypedMutationAndVerificationRemainSeparate(t *testing.T) { + for _, tc := range []struct { + name, operation string + mutate func(*agentexec.ProxmoxGuestLifecycleResultPayload) + }{ + {"reboot status cannot prove restart", "reboot", nil}, + {"completed mutation with failed readback", "shutdown", func(r *agentexec.ProxmoxGuestLifecycleResultPayload) { + r.ExecutionPhase = agentexec.ProxmoxGuestPhaseVerify + r.ReadbackRan = false + r.After = agentexec.ProxmoxGuestLifecycleSnapshot{} + r.Error = "status read timed out" + }}, + } { + t.Run(tc.name, func(t *testing.T) { + now := time.Now().UTC() + h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) + h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now)}}) + agents := &fakeProxmoxActionAgentCommander{mutateResult: tc.mutate} + result, err := newProxmoxGuestActionExecutor(h, agents, nil).ExecuteAction(actionDispatchTestContext(t, "act_vm"), proxmoxGuestActionRecord("act_vm", "vm:160", tc.operation)) + if err != nil { + t.Fatal(err) + } + if result.ActionResultV2.Execution.Status != unified.ActionExecutionSucceeded || result.ActionResultV2.Verification.Status != unified.ActionVerificationInconclusive { + t.Fatalf("execution and verification were conflated: %#v", result) + } + }) + } +} + func TestProxmoxGuestActionExecutorDispatchesVMShutdownAndVerification(t *testing.T) { now := time.Now().UTC() h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) @@ -25,10 +147,7 @@ func TestProxmoxGuestActionExecutorDispatchesVMShutdownAndVerification(t *testin proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now), }, }) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "shutdown requested"}, - {RequestID: "act_vm-verify-1", Success: true, ExitCode: 0, Stdout: "status: stopped"}, - }} + agents := &fakeProxmoxActionAgentCommander{} executor := newProxmoxGuestActionExecutor(h, agents, nil) result, err := executor.ExecuteAction(actionDispatchTestContext(t, "act_vm"), proxmoxGuestActionRecord("act_vm", "vm:160", "shutdown")) @@ -38,20 +157,15 @@ func TestProxmoxGuestActionExecutorDispatchesVMShutdownAndVerification(t *testin if result == nil || !result.Success || result.Verification == nil || !result.Verification.Success { t.Fatalf("result = %#v, want successful execution and verification", result) } - if len(agents.calls) != 2 { - t.Fatalf("agent calls = %d, want dispatch and verification", len(agents.calls)) + if len(agents.calls) != 1 { + t.Fatalf("typed calls = %d, want one dispatch with runner-owned readback", len(agents.calls)) } - if got := agents.calls[0].Command; got != "qm shutdown 160" { - t.Fatalf("dispatch command = %q", got) + call := agents.calls[0] + if call.GuestKind != "vm" || call.Operation != "shutdown" || call.VMID != 160 || call.ExpectedStatus != "running" || call.ActionID != "act_vm" || call.Timeout != 180 || call.RequestID != "act_vm.dispatch.1" { + t.Fatalf("typed dispatch = %#v", call) } - if agents.calls[0].ApprovalID != "act_vm" || !agents.calls[0].Trusted || agents.calls[0].Timeout != 180 { - t.Fatalf("dispatch approval/trust/timeout = %q/%v/%d", agents.calls[0].ApprovalID, agents.calls[0].Trusted, agents.calls[0].Timeout) - } - if agents.calls[0].RequestID != "act_vm.dispatch.1" { - t.Fatalf("dispatch request identity = %q", agents.calls[0].RequestID) - } - if got := agents.calls[1].Command; got != "qm status 160" { - t.Fatalf("verification command = %q", got) + if err := agentexec.ValidateProxmoxGuestLifecyclePayload(&call); err != nil { + t.Fatalf("unbound dispatch: %v", err) } for _, agentID := range agents.callAgents { if agentID != "node-agent-1" { @@ -69,10 +183,7 @@ func TestProxmoxGuestActionExecutorDispatchesLXCStartAndVerification(t *testing. proxmoxGuestActionResource("system-container:101", unified.ResourceTypeSystemContainer, "stopped", now), }, }) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "act_ct", Success: true, ExitCode: 0, Stdout: "start requested"}, - {RequestID: "act_ct-verify-1", Success: true, ExitCode: 0, Stdout: "status: running"}, - }} + agents := &fakeProxmoxActionAgentCommander{} executor := newProxmoxGuestActionExecutor(h, agents, nil) result, err := executor.ExecuteAction(actionDispatchTestContext(t, "act_ct"), proxmoxGuestActionRecord("act_ct", "system-container:101", "start")) @@ -82,15 +193,12 @@ func TestProxmoxGuestActionExecutorDispatchesLXCStartAndVerification(t *testing. if result == nil || !result.Success || result.Verification == nil || !result.Verification.Success { t.Fatalf("result = %#v, want successful execution and verification", result) } - if got := agents.calls[0].Command; got != "pct start 101" { - t.Fatalf("dispatch command = %q", got) - } - if got := agents.calls[1].Command; got != "pct status 101" { - t.Fatalf("verification command = %q", got) + if len(agents.calls) != 1 || agents.calls[0].GuestKind != "ct" || agents.calls[0].VMID != 101 || agents.calls[0].Operation != "start" || agents.calls[0].ExpectedStatus != "stopped" { + t.Fatalf("typed LXC dispatch = %#v", agents.calls) } } -func TestProxmoxGuestActionExecutorResolvesCommandAgentByNodeHostname(t *testing.T) { +func TestProxmoxGuestActionExecutorResolvesTypedRunnerByNodeHostname(t *testing.T) { now := time.Now().UTC() resource := proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now) resource.Proxmox.LinkedAgentID = "stale-agent" @@ -99,11 +207,7 @@ func TestProxmoxGuestActionExecutorResolvesCommandAgentByNodeHostname(t *testing snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{resource}, }) - agents := &fakeDockerActionAgentCommander{ - results: []*agentexec.CommandResultPayload{ - {RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "reboot requested"}, - {RequestID: "act_vm-verify-1", Success: true, ExitCode: 0, Stdout: "status: running"}, - }, + agents := &fakeProxmoxActionAgentCommander{ connected: map[string]bool{ "stale-agent": false, "command-agent-1": true, @@ -146,14 +250,7 @@ func TestProxmoxGuestActionExecutorVerificationContradictionDoesNotRewriteExecut proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now), }, }) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "shutdown requested"}, - {RequestID: "act_vm-verify-1", Success: true, ExitCode: 0, Stdout: "status: running"}, - {RequestID: "act_vm-verify-2", Success: true, ExitCode: 0, Stdout: "status: running"}, - {RequestID: "act_vm-verify-3", Success: true, ExitCode: 0, Stdout: "status: running"}, - {RequestID: "act_vm-verify-4", Success: true, ExitCode: 0, Stdout: "status: running"}, - {RequestID: "act_vm-verify-5", Success: true, ExitCode: 0, Stdout: "status: running"}, - }} + agents := &fakeProxmoxActionAgentCommander{afterStatus: "running"} executor := newProxmoxGuestActionExecutor(h, agents, nil) result, err := executor.ExecuteAction(actionDispatchTestContext(t, "act_vm"), proxmoxGuestActionRecord("act_vm", "vm:160", "shutdown")) @@ -180,10 +277,7 @@ func TestProxmoxGuestActionExecutorUsesIndependentControlPlaneVerification(t *te snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{resource}, }) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "shutdown requested"}, - {RequestID: "act_vm-verify-1", Success: true, ExitCode: 0, Stdout: "status: stopped"}, - }} + agents := &fakeProxmoxActionAgentCommander{} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 3600, "proxmox-control-plane:default:homelab"), // The after observation must postdate actionStartedAt, which is stamped @@ -215,7 +309,7 @@ func TestProxmoxGuestActionExecutorRequiresUptimeResetToVerifyReboot(t *testing. resource.Proxmox.Uptime = 7200 h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{resource}}) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{{RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "reboot requested"}}} + agents := &fakeProxmoxActionAgentCommander{} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 7200, "proxmox-control-plane:default:homelab"), proxmoxGuestActionObservation(now.Add(time.Minute), "running", 4, "proxmox-control-plane:default:homelab"), @@ -239,7 +333,7 @@ func TestProxmoxGuestActionExecutorKeepsIndependentContradictionSeparateFromExec resource := proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now) h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{resource}}) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{{RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "reboot requested"}}} + agents := &fakeProxmoxActionAgentCommander{} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 7200, "proxmox-control-plane:default:homelab"), proxmoxGuestActionObservation(now.Add(time.Minute), "running", 7201, "proxmox-control-plane:default:homelab"), @@ -266,10 +360,7 @@ func TestProxmoxGuestActionExecutorRejectsSameDomainIndependentEvidence(t *testi resource := proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now) h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{resource}}) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "shutdown requested"}, - {RequestID: "act_vm-verify-1", Success: true, ExitCode: 0, Stdout: "status: stopped"}, - }} + agents := &fakeProxmoxActionAgentCommander{} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 100, "agent:node-agent-1"), proxmoxGuestActionObservation(now.Add(time.Second), "stopped", 0, "agent:node-agent-1"), @@ -328,7 +419,7 @@ func TestHandlePlanActionRejectsDisconnectedProxmoxNodeCommandAgent(t *testing.T }) h.SetActionExecutor(newRoutedActionExecutor( h, - newProxmoxGuestActionExecutor(h, &fakeDockerActionAgentCommander{ + newProxmoxGuestActionExecutor(h, &fakeProxmoxActionAgentCommander{ connected: map[string]bool{"node-agent-1": false}, }, nil), )) @@ -347,8 +438,8 @@ func TestHandlePlanActionRejectsDisconnectedProxmoxNodeCommandAgent(t *testing.T t.Fatalf("plan status = %d, want %d, body=%s", rec.Code, http.StatusConflict, rec.Body.String()) } if !strings.Contains(rec.Body.String(), `"error":"action_execution_unavailable"`) || - !strings.Contains(rec.Body.String(), `"reason":"Proxmox node command agent is not connected."`) || - !strings.Contains(rec.Body.String(), `"reasonCode":"command_agent_disconnected"`) { + !strings.Contains(rec.Body.String(), `"reason":"Connect a typed action runner on this Proxmox node before planning a guest action."`) || + !strings.Contains(rec.Body.String(), `"reasonCode":"action_runner_unavailable"`) { t.Fatalf("unexpected response body: %s", rec.Body.String()) } store, err := h.getStore("default") @@ -375,7 +466,7 @@ func TestResourceResponsesFilterDisconnectedProxmoxLifecycleCapabilities(t *test }) h.SetActionExecutor(newRoutedActionExecutor( h, - newProxmoxGuestActionExecutor(h, &fakeDockerActionAgentCommander{ + newProxmoxGuestActionExecutor(h, &fakeProxmoxActionAgentCommander{ connected: map[string]bool{"node-agent-1": false}, }, nil), )) @@ -397,7 +488,7 @@ func TestResourceResponsesFilterDisconnectedProxmoxLifecycleCapabilities(t *test t.Fatalf("list capabilities = %#v, want none", got) } readiness, ok := dockerActionReadinessByName(list.Data[0].ActionReadiness, "reboot") - if !ok || readiness.Available || readiness.ReasonCode != "command_agent_disconnected" { + if !ok || readiness.Available || readiness.ReasonCode != "action_runner_unavailable" { t.Fatalf("list action readiness = %#v, ok=%v; want disconnected reboot", list.Data[0].ActionReadiness, ok) } } @@ -510,3 +601,45 @@ func proxmoxGuestActionRecord(actionID, resourceID, operation string) unified.Ac }, } } + +func TestProxmoxTypedReadbackClockSkewPreservesExecutionAndEvidence(t *testing.T) { + for _, tc := range []struct { + name string + skew time.Duration + verified bool + }{ + {"ahead within bound", 2 * time.Second, true}, + {"behind within bound", -2 * time.Second, true}, + {"excessively ahead", 6 * time.Minute, false}, + {"stale", -16 * time.Minute, false}, + } { + t.Run(tc.name, func(t *testing.T) { + now := time.Now().UTC() + observedAt := now.Add(tc.skew) + h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) + h.SetStateProvider(resourceUnifiedSeedProvider{snapshot: models.StateSnapshot{LastUpdate: now}, resources: []unified.Resource{proxmoxGuestActionResource("vm:160", unified.ResourceTypeVM, "running", now)}}) + agents := &fakeProxmoxActionAgentCommander{mutateResult: func(r *agentexec.ProxmoxGuestLifecycleResultPayload) { + r.After.ObservedAt = observedAt + }} + result, err := newProxmoxGuestActionExecutor(h, agents, nil).ExecuteAction(actionDispatchTestContext(t, "act_clock"), proxmoxGuestActionRecord("act_clock", "vm:160", "shutdown")) + if err != nil { + t.Fatalf("clock skew discarded execution result: %v", err) + } + truth := result.ActionResultV2 + if truth.Execution.Status != unified.ActionExecutionSucceeded { + t.Fatalf("readback clock rewrote completed execution: %#v", truth) + } + if tc.verified { + if truth.Verification.Status != unified.ActionVerificationConfirmed || truth.Verification.EvidenceClass != unified.ActionEvidenceAgentAttested || len(truth.Verification.Evidence) != 1 { + t.Fatalf("bounded skew lost readback: %#v", truth) + } + e := truth.Verification.Evidence[0] + if !e.ObservedAt.Equal(observedAt) || e.ReceivedAt.Before(now) || e.ReceivedAt.After(time.Now().UTC()) { + t.Fatalf("observation/receipt clocks were rewritten: %#v", e) + } + } else if truth.Verification.Status != unified.ActionVerificationInconclusive || truth.Verification.ReasonCode != "stale_agent_readback" || len(truth.Verification.Evidence) != 0 { + t.Fatalf("unusable clock established verification: %#v", truth) + } + }) + } +} diff --git a/internal/api/proxmox_guest_action_result.go b/internal/api/proxmox_guest_action_result.go index d9861cc0c..99a545bc4 100644 --- a/internal/api/proxmox_guest_action_result.go +++ b/internal/api/proxmox_guest_action_result.go @@ -10,7 +10,7 @@ import ( unified "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) -func proxmoxGuestExecutionResult(actionID, resourceID, agentID string, kind proxmoxGuestKind, operation string, exitCode int, output, commandError string, agentVerification *unified.ActionVerificationResult, independentBefore, independentAfter *proxmoxGuestPostconditionObservation, independentEvaluation agentexec.PostconditionEvaluation, actionStartedAt time.Time) (*unified.ExecutionResult, error) { +func proxmoxGuestExecutionResult(actionID, resourceID, agentID string, kind proxmoxGuestKind, operation string, exitCode int, output, commandError string, agentVerification *unified.ActionVerificationResult, independentBefore, independentAfter *proxmoxGuestPostconditionObservation, independentEvaluation agentexec.PostconditionEvaluation, actionStartedAt, agentReceivedAt time.Time) (*unified.ExecutionResult, error) { executionSummary := fmt.Sprintf("Proxmox guest %s command exited with status %d.", strings.TrimSpace(operation), exitCode) execution := unified.ActionExecutionTruth{Status: unified.ActionExecutionSucceeded, Summary: executionSummary} verification := unified.ActionVerificationTruth{Status: unified.ActionVerificationInconclusive, EvidenceClass: unified.ActionEvidenceNone, ReasonCode: "agent_readback_inconclusive", Summary: "The executing agent did not return a conclusive postcondition read."} @@ -21,6 +21,9 @@ func proxmoxGuestExecutionResult(actionID, resourceID, agentID string, kind prox execution.Summary = strings.TrimSpace(firstNonEmpty(commandError, output, executionSummary)) verification = unified.ActionVerificationTruth{Status: unified.ActionVerificationNotAttempted, EvidenceClass: unified.ActionEvidenceNone} legacy.ErrorMessage = execution.Summary + } else if agentVerification != nil && agentVerification.Ran && !freshProxmoxAgentReadback(agentVerification.RanAt, agentReceivedAt) { + verification.ReasonCode = "stale_agent_readback" + verification.Summary = "The executing agent readback had missing, stale, or excessively skewed timestamps." } else if agentVerification != nil && agentVerification.Ran { agentSummary := strings.TrimSpace(firstNonEmpty(agentVerification.Note, agentVerification.Output, "The executing agent read the Proxmox guest postcondition.")) evidence, err := unified.NormalizeActionEvidence(unified.ActionEvidence{ @@ -33,7 +36,7 @@ func proxmoxGuestExecutionResult(actionID, resourceID, agentID string, kind prox Method: "server_owned_proxmox_cli_status", SubjectID: resourceID, ObservedAt: agentVerification.RanAt.UTC(), - ReceivedAt: time.Now().UTC(), + ReceivedAt: agentReceivedAt.UTC(), Summary: agentSummary, }) if err != nil { @@ -125,3 +128,12 @@ func usableIndependentProxmoxBeforeObservation(actionStartedAt time.Time, observ receivedAt := observation.ReceivedAt.UTC() return !observedAt.After(actionStartedAt.UTC()) && !observedAt.After(receivedAt.Add(5*time.Minute)) && !observedAt.Before(receivedAt.Add(-15*time.Minute)) } + +// A request-bound runner observation may use a different clock. Freshness +// limits qualify the readback without rewriting either clock or discarding +// completed execution when only its verification is unusable. +func freshProxmoxAgentReadback(observedAt, receivedAt time.Time) bool { + return !observedAt.IsZero() && !receivedAt.IsZero() && + !observedAt.Before(receivedAt.Add(-15*time.Minute)) && + !observedAt.After(receivedAt.Add(5*time.Minute)) +} diff --git a/internal/api/proxmox_patrol_integration_test.go b/internal/api/proxmox_patrol_integration_test.go index b3b7aef69..3a36666af 100644 --- a/internal/api/proxmox_patrol_integration_test.go +++ b/internal/api/proxmox_patrol_integration_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/rcourtman/pulse-go-rewrite/internal/agentexec" "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" @@ -40,10 +39,7 @@ func TestProxmoxStoppedDetectorProposalApprovalDispatchIndependentVerificationAn snapshot: models.StateSnapshot{LastUpdate: stopped.LastSeen}, resources: []unified.Resource{stopped}, }) - agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{ - {RequestID: "dispatch", Success: true, ExitCode: 0, Stdout: "start requested"}, - {RequestID: "verify", Success: true, ExitCode: 0, Stdout: "status: running"}, - }} + agents := &fakeProxmoxActionAgentCommander{} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "stopped", 0, "proxmox-control-plane:default:homelab"), proxmoxGuestActionObservation(now.Add(time.Minute), "running", 5, "proxmox-control-plane:default:homelab"), @@ -109,7 +105,7 @@ func TestProxmoxStoppedDetectorProposalApprovalDispatchIndependentVerificationAn if truth.Verification.Evidence[0].ObserverKind != "proxmox_control_plane" || truth.Verification.Evidence[0].ObserverTrustDomain == truth.Verification.Evidence[0].ExecutorTrustDomain { t.Fatalf("independent evidence=%#v", truth.Verification.Evidence[0]) } - if len(agents.calls) != 2 || agents.calls[0].Command != "qm start 160" || agents.calls[1].Command != "qm status 160" { + if len(agents.calls) != 1 || agents.calls[0].GuestKind != "vm" || agents.calls[0].Operation != "start" || agents.calls[0].VMID != 160 { t.Fatalf("typed Proxmox calls=%#v", agents.calls) } diff --git a/internal/api/router.go b/internal/api/router.go index 6cb59249f..a9f60cb2b 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -4460,7 +4460,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { } // Dev mode bypass for admin endpoints (disabled by default) - if adminBypassEnabled() { + if adminBypassAppliesToRequest(req) { log.Debug(). Str("path", req.URL.Path). Msg("Admin bypass enabled - skipping global auth") diff --git a/internal/api/security_test.go b/internal/api/security_test.go index 1d605ff28..d69203a14 100644 --- a/internal/api/security_test.go +++ b/internal/api/security_test.go @@ -2083,6 +2083,12 @@ func TestRequireAdmin_ProxyAuthTrimSpacesInRoles(t *testing.T) { } func TestRequireAdmin_NoProxyAuthAuthenticatedAllowed(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + resetSessionStoreForTests() + t.Cleanup(resetSessionStoreForTests) + InitSessionStore(t.TempDir()) + // When proxy auth is not configured, authenticated users are considered admins hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ @@ -2185,6 +2191,9 @@ func TestRequireAuth_APIOnlyModeAcceptsValidToken(t *testing.T) { } func TestRequireAuth_InvalidBasicAuthRejectsRequest(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ AuthUser: "testuser", @@ -2210,6 +2219,9 @@ func TestRequireAuth_InvalidBasicAuthRejectsRequest(t *testing.T) { } func TestRequireAuth_InvalidBasicAuthAPIPathReturnsJSON(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ AuthUser: "testuser", @@ -2235,6 +2247,9 @@ func TestRequireAuth_InvalidBasicAuthAPIPathReturnsJSON(t *testing.T) { } func TestRequireAuth_InvalidBasicAuthAcceptJSONReturnsJSON(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ AuthUser: "testuser", @@ -2258,6 +2273,9 @@ func TestRequireAuth_InvalidBasicAuthAcceptJSONReturnsJSON(t *testing.T) { } func TestRequireAuth_InvalidBasicAuthNonAPIReturnsPlainText(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ AuthUser: "testuser", @@ -2282,6 +2300,12 @@ func TestRequireAuth_InvalidBasicAuthNonAPIReturnsPlainText(t *testing.T) { } func TestRequireAuth_ValidBasicAuthAllowsAccess(t *testing.T) { + resetFailedLogins() + t.Cleanup(resetFailedLogins) + resetSessionStoreForTests() + t.Cleanup(resetSessionStoreForTests) + InitSessionStore(t.TempDir()) + hashedPass, _ := auth.HashPassword("password123") cfg := &config.Config{ AuthUser: "testuser", diff --git a/internal/unifiedresources/action_result_v2.go b/internal/unifiedresources/action_result_v2.go index b23c74ce0..49ed62a75 100644 --- a/internal/unifiedresources/action_result_v2.go +++ b/internal/unifiedresources/action_result_v2.go @@ -255,9 +255,9 @@ func normalizeActionEvidenceFields(evidence ActionEvidence) (ActionEvidence, err } evidence.ObservedAt = evidence.ObservedAt.UTC() evidence.ReceivedAt = evidence.ReceivedAt.UTC() - if evidence.ReceivedAt.Before(evidence.ObservedAt) { - return ActionEvidence{}, fmt.Errorf("%w: receivedAt predates observedAt", ErrInvalidActionEvidence) - } + // Observer and receiver timestamps belong to separate clocks. Preserve + // both facts without imposing cross-clock ordering or rewriting either. + // Capability-specific verification owns freshness and clock-skew limits. if evidence.ID == "" || evidence.ObserverID == "" || evidence.ObserverKind == "" || evidence.ObserverTrustDomain == "" || evidence.ExecutorTrustDomain == "" || evidence.Method == "" || evidence.SubjectID == "" { return ActionEvidence{}, fmt.Errorf("%w: evidence identity, observer kind, trust domains, method, and subject required", ErrInvalidActionEvidence) } diff --git a/internal/unifiedresources/action_result_v2_test.go b/internal/unifiedresources/action_result_v2_test.go index c726c9a7b..4edff187f 100644 --- a/internal/unifiedresources/action_result_v2_test.go +++ b/internal/unifiedresources/action_result_v2_test.go @@ -462,3 +462,31 @@ func TestCompensationStateShapeValidationMatrix(t *testing.T) { }) } } + +func TestActionEvidencePreservesSeparateObserverAndReceiverClocks(t *testing.T) { + for _, skew := range []time.Duration{-2 * time.Second, 0, 2 * time.Second} { + t.Run(skew.String(), func(t *testing.T) { + evidence := actionResultTestEvidence(ActionEvidenceAgentAttested)[0] + evidence.ObservedAt = evidence.ReceivedAt.Add(skew) + got, err := NormalizeActionEvidence(evidence) + if err != nil { + t.Fatal(err) + } + if !got.ObservedAt.Equal(evidence.ObservedAt) || !got.ReceivedAt.Equal(evidence.ReceivedAt) { + t.Fatalf("cross-clock evidence was rewritten: %#v", got) + } + changed := got + changed.Digest = "" + changed.ObservedAt = changed.ObservedAt.Add(time.Millisecond) + digest, err := ActionEvidenceDigest(changed) + if err != nil || digest == got.Digest { + t.Fatalf("observation clock is not bound to digest: %s, %v", digest, err) + } + changed = got + changed.ReceivedAt = time.Time{} + if _, err := NormalizeActionEvidence(changed); !errors.Is(err, ErrInvalidActionEvidence) { + t.Fatalf("missing receipt time was accepted: %v", err) + } + }) + } +}