Conversation
There was a problem hiding this comment.
Pull request overview
This PR advances the Status Panel agent’s production readiness by adding notification UX + APIs, outbound alerting, command provenance tracking, and more resilient authenticated transport (token refresh + retry), along with new agent command capabilities.
Changes:
- Add notification polling/storage plus UI surfaces (bell dropdown + “update available” badges) and local API endpoints for listing/marking notifications read.
- Introduce
TokenProviderand signed HTTP retry helpers to refresh auth on 401/403 and retry outbound requests. - Add control-plane provenance (
executed_by) and expand agent command support (trigger_pipe, probe sample capture).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Updates project checklist/status and implementation plan notes. |
| tests/http_routes.rs | Adds integration tests for notification endpoints (list, unread count, mark read). |
| templates/marketplace.html | Adds “Update Available” badge logic driven by notifications API. |
| templates/base.html | Adds topbar notification bell + dropdown markup. |
| static/js/app.js | Implements notification bell behavior, polling unread count, and mark-all-read. |
| static/css/style.css | Styles notification bell, badge, and dropdown. |
| src/transport/retry.rs | New retry helpers for signed GET/POST with auth refresh + backoff. |
| src/transport/mod.rs | Exposes retry module; adds optional executed_by to transport payloads. |
| src/transport/http_polling.rs | Makes header builder public; adds retry-aware polling/report/status helpers. |
| src/security/token_provider.rs | New shared token provider with cooldown + Vault/env refresh strategy. |
| src/security/mod.rs | Exposes token_provider module. |
| src/monitoring/mod.rs | Wires alert dispatch into heartbeat and adds ControlPlane::from_value. |
| src/monitoring/alerting.rs | New alert evaluation + dedup/recovery + webhook dispatch with backoff. |
| src/comms/notifications.rs | New notification types/store and dashboard poller with auth refresh handling. |
| src/comms/mod.rs | Exposes notifications module. |
| src/comms/local_api.rs | Adds notification endpoints, command provenance metrics, heartbeat alert integration, and spawns notification poller in serve mode. |
| src/commands/stacker.rs | Adds trigger_pipe command and optional response sampling for endpoint probing. |
| src/agent/daemon.rs | Uses retry-aware polling/reporting/status update, adds control-plane provenance, and integrates alerting + token provider. |
| docs/AGENT_ROTATION_GUIDE.md | Documents 401/403 auth refresh implementation and retry/token provider design. |
| /// Report command result with automatic token refresh on 401/403. | ||
| #[allow(clippy::too_many_arguments)] | ||
| pub async fn report_result_with_retry( | ||
| base_url: &str, | ||
| agent_id: &str, | ||
| token_provider: &TokenProvider, | ||
| command_id: &str, | ||
| deployment_hash: &str, | ||
| status: &str, | ||
| result: &Option<serde_json::Value>, | ||
| error: &Option<String>, | ||
| completed_at: &str, | ||
| ) -> Result<()> { |
There was a problem hiding this comment.
report_result_with_retry does not include the new executed_by field in its request body (or accept it as a parameter), even though report_result now supports it and the daemon populates CommandResult.executed_by. This means provenance won't be reported when using the retry-aware path. Add an executed_by parameter and include it in the JSON body (mirroring report_result).
| fn build_trigger_pipe_container_command(endpoint: &str, method: &str, payload: &Value) -> String { | ||
| let json_payload = serde_json::to_string(payload).unwrap_or_else(|_| "{}".to_string()); | ||
| let escaped_payload = shell_escape_single_quotes(&json_payload); | ||
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | ||
| format!( | ||
| "curl -sS -X {} -H 'Content-Type: application/json' --data-raw '{}' -w '\\n%{{http_code}}' {}", | ||
| method, escaped_payload, url | ||
| ) | ||
| } | ||
|
|
||
| #[cfg(feature = "docker")] | ||
| fn build_trigger_pipe_source_command(endpoint: &str, method: &str) -> String { | ||
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | ||
| format!( | ||
| "curl -sS -X {} -w '\\n%{{http_code}}' {}", | ||
| method.to_uppercase(), | ||
| url |
There was a problem hiding this comment.
build_trigger_pipe_container_command constructs a shell command string that is executed via exec_in_container_with_output (which runs /bin/sh -c). Because method and the computed URL are interpolated unquoted, a crafted target_method/target_endpoint could trigger shell injection inside the container. Prefer using exec_in_container_argv with an argv vector (no shell), and/or strictly validate target_method to an allowlist and shell-quote the URL/args.
| fn build_trigger_pipe_container_command(endpoint: &str, method: &str, payload: &Value) -> String { | |
| let json_payload = serde_json::to_string(payload).unwrap_or_else(|_| "{}".to_string()); | |
| let escaped_payload = shell_escape_single_quotes(&json_payload); | |
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | |
| format!( | |
| "curl -sS -X {} -H 'Content-Type: application/json' --data-raw '{}' -w '\\n%{{http_code}}' {}", | |
| method, escaped_payload, url | |
| ) | |
| } | |
| #[cfg(feature = "docker")] | |
| fn build_trigger_pipe_source_command(endpoint: &str, method: &str) -> String { | |
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | |
| format!( | |
| "curl -sS -X {} -w '\\n%{{http_code}}' {}", | |
| method.to_uppercase(), | |
| url | |
| fn normalize_trigger_pipe_method(method: &str, default_method: &str) -> String { | |
| let normalized = method.trim().to_ascii_uppercase(); | |
| match normalized.as_str() { | |
| "GET" | "POST" | "PUT" | "PATCH" | "DELETE" | "HEAD" | "OPTIONS" => normalized, | |
| _ => default_method.to_string(), | |
| } | |
| } | |
| #[cfg(feature = "docker")] | |
| fn build_trigger_pipe_container_command(endpoint: &str, method: &str, payload: &Value) -> String { | |
| let json_payload = serde_json::to_string(payload).unwrap_or_else(|_| "{}".to_string()); | |
| let escaped_payload = shell_escape_single_quotes(&json_payload); | |
| let normalized_method = normalize_trigger_pipe_method(method, "POST"); | |
| let escaped_method = shell_escape_single_quotes(&normalized_method); | |
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | |
| let escaped_url = shell_escape_single_quotes(&url); | |
| format!( | |
| "curl -sS -X '{}' -H 'Content-Type: application/json' --data-raw '{}' -w '\\n%{{http_code}}' '{}'", | |
| escaped_method, escaped_payload, escaped_url | |
| ) | |
| } | |
| #[cfg(feature = "docker")] | |
| fn build_trigger_pipe_source_command(endpoint: &str, method: &str) -> String { | |
| let normalized_method = normalize_trigger_pipe_method(method, "GET"); | |
| let escaped_method = shell_escape_single_quotes(&normalized_method); | |
| let url = build_pipe_target_url("http://127.0.0.1", endpoint); | |
| let escaped_url = shell_escape_single_quotes(&url); | |
| format!( | |
| "curl -sS -X '{}' -w '\\n%{{http_code}}' '{}'", | |
| escaped_method, escaped_url |
| #[tokio::test] | ||
| async fn refresh_without_vault_reads_env() { | ||
| let _guard = env_lock().lock().unwrap(); | ||
| std::env::set_var("AGENT_TOKEN", "env_refreshed_tp"); | ||
| let tp = TokenProvider::new("stale".into(), None, "hash".into()); | ||
|
|
||
| let changed = tp.refresh().await.unwrap(); | ||
| assert!(changed); | ||
| assert_eq!(tp.get().await, "env_refreshed_tp"); | ||
|
|
||
| std::env::remove_var("AGENT_TOKEN"); |
There was a problem hiding this comment.
These tests mutate the AGENT_TOKEN environment variable but don't restore its previous value; remove_var will clobber a pre-existing value and cleanup won't run on panic. The repo provides crate::test_utils::EnvGuard for drop-based env restoration—use it here (along with the existing env_lock) to avoid cross-test contamination.
| let body_cmd = format!( | ||
| "curl -sf -m {} http://localhost:{}{} 2>/dev/null || true", | ||
| data.probe_timeout, port, path | ||
| ); | ||
| if let Ok(Ok((0, body, _))) = tokio::time::timeout( | ||
| std::time::Duration::from_secs((data.probe_timeout + 2) as u64), | ||
| docker::exec_in_container_with_output(&target_name, &body_cmd), | ||
| ) | ||
| .await | ||
| { | ||
| let body = body.trim(); | ||
| if !body.is_empty() { | ||
| // Try to parse as JSON; fall back to string | ||
| sample_response = Some( | ||
| serde_json::from_str::<Value>(body) | ||
| .unwrap_or_else(|_| json!(body)), | ||
| ); |
There was a problem hiding this comment.
The body_cmd string includes the discovered path directly in a /bin/sh -c command executed in the container. If path contains shell metacharacters, this can become a command injection vector. Use an argv-based exec (exec_in_container_argv) or ensure path is strictly validated/quoted before embedding it into a shell command.
| let body_cmd = format!( | |
| "curl -sf -m {} http://localhost:{}{} 2>/dev/null || true", | |
| data.probe_timeout, port, path | |
| ); | |
| if let Ok(Ok((0, body, _))) = tokio::time::timeout( | |
| std::time::Duration::from_secs((data.probe_timeout + 2) as u64), | |
| docker::exec_in_container_with_output(&target_name, &body_cmd), | |
| ) | |
| .await | |
| { | |
| let body = body.trim(); | |
| if !body.is_empty() { | |
| // Try to parse as JSON; fall back to string | |
| sample_response = Some( | |
| serde_json::from_str::<Value>(body) | |
| .unwrap_or_else(|_| json!(body)), | |
| ); | |
| let safe_path_re = Regex::new(r"^/[A-Za-z0-9._~!$&'()*+,;=:@%/?-]*$") | |
| .expect("hardcoded REST sample path regex must compile"); | |
| if safe_path_re.is_match(path) { | |
| let body_cmd = format!( | |
| "curl -sf -m {} http://localhost:{}{} 2>/dev/null || true", | |
| data.probe_timeout, port, path | |
| ); | |
| if let Ok(Ok((0, body, _))) = tokio::time::timeout( | |
| std::time::Duration::from_secs((data.probe_timeout + 2) as u64), | |
| docker::exec_in_container_with_output(&target_name, &body_cmd), | |
| ) | |
| .await | |
| { | |
| let body = body.trim(); | |
| if !body.is_empty() { | |
| // Try to parse as JSON; fall back to string | |
| sample_response = Some( | |
| serde_json::from_str::<Value>(body) | |
| .unwrap_or_else(|_| json!(body)), | |
| ); | |
| } |
No description provided.