feat: ship HTTP dashboard and harden daemon/API flows

Add the static HTTP dashboard example and wire in the recent daemon/API polish:
CORS-aware API routing, service-install behavior cleanup, safer systemd unit
ExecStart quoting, and friendly-name validation for path-safe targeting.

Also refresh README/API/roadmap docs, remove the temporary claude observations
file, and include the related tests for API/status and daemon validation.
This commit is contained in:
44r0n7
2026-04-18 16:45:12 -04:00
parent 795aa2f713
commit 59fb56558f
17 changed files with 999 additions and 726 deletions
Generated
+1
View File
@@ -1706,6 +1706,7 @@ dependencies = [
"thiserror", "thiserror",
"tokio", "tokio",
"toml", "toml",
"tower-http",
"tracing", "tracing",
"tracing-subscriber", "tracing-subscriber",
"urlencoding", "urlencoding",
+1
View File
@@ -18,6 +18,7 @@ serde_json = "1.0"
thiserror = "2.0" thiserror = "2.0"
tokio = { version = "1.0", features = ["full"] } tokio = { version = "1.0", features = ["full"] }
toml = "0.8" toml = "0.8"
tower-http = { version = "0.6", features = ["cors"] }
tracing = "0.1" tracing = "0.1"
tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] } tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] }
urlencoding = "2.1" urlencoding = "2.1"
+2
View File
@@ -42,6 +42,8 @@ tvctl/
│ ├── DESIGN.md ← Full design decisions and rationale │ ├── DESIGN.md ← Full design decisions and rationale
│ ├── API.md ← HTTP API specification (detailed) │ ├── API.md ← HTTP API specification (detailed)
│ └── ADAPTER.md ← Adapter trait spec and implementation guide │ └── ADAPTER.md ← Adapter trait spec and implementation guide
├── examples/
│ └── http-dashboard/ ← Static browser dashboard for /v1 API endpoint testing
├── src/ ├── src/
│ ├── lib.rs ← Library surface for shared modules and integration tests │ ├── lib.rs ← Library surface for shared modules and integration tests
│ ├── main.rs ← Binary entry point and runtime bootstrap │ ├── main.rs ← Binary entry point and runtime bootstrap
+45
View File
@@ -90,6 +90,16 @@ builders — Home Assistant integrations, stream deck plugins, scripts, dashboar
default. HTTP handlers dispatch directly inside the daemon process rather than looping back through default. HTTP handlers dispatch directly inside the daemon process rather than looping back through
the Unix socket. All responses are JSON. the Unix socket. All responses are JSON.
Browser clients can be enabled with opt-in CORS settings:
```toml
[daemon]
cors_enabled = true
cors_allowed_origins = ["http://127.0.0.1:8080", "http://localhost:8080"]
```
A full endpoint test dashboard example lives at `examples/http-dashboard/`.
**Adapter Layer** **Adapter Layer**
Each TV platform is implemented as an adapter behind a common Rust trait. The adapter handles all Each TV platform is implemented as an adapter behind a common Rust trait. The adapter handles all
platform-specific protocol details. Above the adapter layer, nothing knows or cares whether the TV platform-specific protocol details. Above the adapter layer, nothing knows or cares whether the TV
@@ -300,6 +310,20 @@ GET /v1/devices/a3f2c1d4-.../state
GET /v1/devices/living-room/state ← same result GET /v1/devices/living-room/state ← same result
``` ```
If you use a friendly name in the path, URL-encode it first. This is required
for names containing spaces or reserved characters.
```bash
# Friendly name: "Living Room TV"
curl http://127.0.0.1:7272/v1/devices/Living%20Room%20TV/state
```
Friendly-name validation:
- Spaces are allowed.
- Leading/trailing whitespace is trimmed.
- `/` and `%` are rejected to keep path-based targeting safe and unambiguous.
### Example Requests ### Example Requests
```bash ```bash
@@ -381,6 +405,8 @@ socket = "/run/user/1000/tvctl.sock"
http_enabled = true http_enabled = true
http_port = 7272 http_port = 7272
http_host = "127.0.0.1" http_host = "127.0.0.1"
cors_enabled = false
cors_allowed_origins = []
log_level = "info" log_level = "info"
[discovery] [discovery]
@@ -405,6 +431,8 @@ roku_password = ""
| `daemon.http_enabled` | `true` | Expose HTTP API | | `daemon.http_enabled` | `true` | Expose HTTP API |
| `daemon.http_port` | `7272` | HTTP API port | | `daemon.http_port` | `7272` | HTTP API port |
| `daemon.http_host` | `127.0.0.1` | Bind address (loopback by default) | | `daemon.http_host` | `127.0.0.1` | Bind address (loopback by default) |
| `daemon.cors_enabled` | `false` | Enable browser CORS responses for allowed origins |
| `daemon.cors_allowed_origins` | `[]` | Explicit CORS origin allowlist used when CORS is enabled |
| `daemon.log_level` | `info` | Tracing filter (for example `info` or `tvctl=debug`), applied on daemon start and `tvctl config reload` | | `daemon.log_level` | `info` | Tracing filter (for example `info` or `tvctl=debug`), applied on daemon start and `tvctl config reload` |
| `discovery.auto_discover` | `true` | Discover devices on daemon start | | `discovery.auto_discover` | `true` | Discover devices on daemon start |
| `discovery.interval_secs` | `300` | Re-scan interval (0 = disabled) | | `discovery.interval_secs` | `300` | Re-scan interval (0 = disabled) |
@@ -419,6 +447,23 @@ roku_password = ""
## Storage Layout ## Storage Layout
``` ```
## Dashboard Example
A static browser dashboard for manually testing every API endpoint is included:
`examples/http-dashboard/`
Run it:
```bash
tvctl config set daemon.cors_enabled true
tvctl config set daemon.cors_allowed_origins "http://127.0.0.1:8080,http://localhost:8080"
tvctl config reload
python3 -m http.server 8080 -d examples/http-dashboard
```
Open `http://127.0.0.1:8080`.
~/.config/tvctl/ ~/.config/tvctl/
└── config.toml └── config.toml
+2 -1
View File
@@ -117,13 +117,14 @@ _Goal: Full /v1/ API running on 127.0.0.1:7272._
_Goal: Ready for real use._ _Goal: Ready for real use._
- [x] 2026-04-15 — Shell completions (bash, zsh, fish) via clap - [x] 2026-04-15 — Shell completions (bash, zsh, fish) via clap
- [ ] `tvctl daemon install` generates correct systemd unit file - [x] 2026-04-18 — `tvctl daemon install` generates correct systemd unit file
- [x] 2026-04-15 — First-run experience: helpful output when no devices discovered yet - [x] 2026-04-15 — First-run experience: helpful output when no devices discovered yet
- [x] 2026-04-15 — Daemon startup message with socket path and HTTP port - [x] 2026-04-15 — Daemon startup message with socket path and HTTP port
- [x] 2026-04-18 — Log output via `tracing` (respects `log_level` config) - [x] 2026-04-18 — Log output via `tracing` (respects `log_level` config)
- [x] 2026-04-15 — README accuracy pass (verify all examples work) - [x] 2026-04-15 — README accuracy pass (verify all examples work)
- [x] 2026-04-15 — `cargo clippy` clean - [x] 2026-04-15 — `cargo clippy` clean
- [x] 2026-04-15 — `cargo test` passing - [x] 2026-04-15 — `cargo test` passing
- [x] 2026-04-18 — Included static HTTP dashboard example for endpoint-by-endpoint API testing
- [ ] Cross-compile test (x86_64 + aarch64) - [ ] Cross-compile test (x86_64 + aarch64)
- [ ] GitHub Actions CI (build + clippy + test, Rust toolchain >= 1.85 for edition 2024) - [ ] GitHub Actions CI (build + clippy + test, Rust toolchain >= 1.85 for edition 2024)
- [ ] First binary release - [ ] First binary release
-716
View File
@@ -1,716 +0,0 @@
AGENT INSTRUCTION: BELOW ARE SOME OBSERVATIONS FROM CLAUDE. ITS ONLY PARTIAL BECAUSE THE FREE PLAN LIMIT WAS REACHED. BUT READ THIS AND LEARN WHAT YOU CAN ABOUT IT. THEN IMPLEMENT SOME FIXES ETC AFTER VALIDATING THE ISSUE.
Now I have a thorough picture of the entire codebase. Let me compile my findings.---
Here's a thorough analysis of the project with agent instructions ready for use.
---
## Project Health Summary
**On track**: The project is genuinely following its plan. Milestones 15 are complete. Milestone 6 is partially done. Architecture, data shapes, CLI surface, IPC protocol, and adapter trait are all correctly implemented. The `node-compile-cache/` and `phantomjs/` directories in the archive are clearly accidental inclusions from the working directory — they should not be in the repo.
---
## Agent Instructions
```
You are working on the `tvctl` Rust project. Read PROJECT_MAP.md first.
Do NOT rewrite files wholesale. Make targeted, surgical changes only.
Do NOT touch anything not explicitly listed below.
Run `cargo clippy` and `cargo test` after each change and fix any new warnings before moving on.
---
### 1. CRITICAL BUG — `format_tv_key` duplicates `parse_normalized_tv_key` in reverse
File: `src/daemon/mod.rs`, function `format_tv_key`
This function is a full 33-arm hand-written match over `TvKey` that returns the kebab-case string for each key.
The `TvKey` enum in `src/adapters/mod.rs` is already decorated with `#[serde(rename_all = "kebab-case")]`.
`format_tv_key` can never drift from the parse table without silently producing wrong values. It is
duplication by definition and contains a latent inconsistency: if a new `TvKey` variant is added, it
must be added in three places (the enum, `parse_normalized_tv_key`, AND `format_tv_key`) with no
compiler enforcement that the last one is updated.
**Fix**: Delete `format_tv_key` and replace its single call site with a serde round-trip:
```rust
// In place of: format_tv_key(&key)
// Use:
serde_json::to_value(&key)
.ok()
.and_then(|v| v.as_str().map(str::to_string))
.unwrap_or_else(|| format!("{key:?}"))
```
Or, alternatively, implement `std::fmt::Display` for `TvKey` in `src/adapters/mod.rs` using serde's
rename logic, and use that in the call site. Either approach eliminates the duplicate table.
---
### 2. BUG — `DeviceRegistry::remove` duplicates the match logic of `DeviceRegistry::find`
File: `src/daemon/registry.rs`
`find` uses an inline closure:
```rust
self.devices.iter().find(|device| {
target_uuid.map(|uuid| device.id == uuid).unwrap_or(false)
|| device.name.to_ascii_lowercase() == normalized
})
```
`remove` uses a separate private `matches_target` free function with identical logic.
These two code paths will diverge if the matching rules ever change (e.g., adding prefix-match support).
**Fix**: Refactor `remove` to use `find` to locate the index first:
```rust
pub fn remove(&mut self, target: &str) -> Option<Device> {
let id = self.find(target)?.id;
let index = self.devices.iter().position(|d| d.id == id)?;
let removed = self.devices.remove(index);
self.ensure_default();
Some(removed)
}
```
Then delete the `matches_target` free function entirely.
---
### 3. BUG — `parse_bool` is a useless wrapper in `src/daemon/config.rs`
File: `src/daemon/config.rs`, lines ~300303
```rust
fn parse_bool(key: &str, value: &str) -> anyhow::Result<bool> {
parse_value(key, value)
}
```
This is a zero-value indirection. `parse_value::<bool>` already exists and already handles `bool`.
`parse_bool` exists solely because someone called it before `parse_value` was made generic.
**Fix**: Delete `parse_bool`. Replace the three call sites with `parse_value::<bool>(key, value)?`
(or just `parse_value(key, value)?` — Rust will infer the type from the assignment target).
---
### 4. ARCHITECTURE CONCERN — HTTP API routes through the Unix socket unnecessarily
File: `src/api/mod.rs`, function `send_daemon_request`
The HTTP API runs *inside the same process as the daemon*. It already holds `SharedDaemon: Arc<Mutex<Daemon>>`.
Despite this, every HTTP handler opens a new Unix socket connection back to the daemon, serializes
the request to JSON, sends it over the socket, and deserializes the response back. This is:
- An unnecessary round-trip through the OS socket layer
- A source of failure (`daemon_socket_unreachable`) for requests that should never fail
- A potential deadlock: the HTTP handler acquires the Mutex briefly to get the socket path, releases it,
then the request comes back in on a socket connection and tries to acquire the Mutex in `execute_request`.
This works today because `handle_connection` holds no lock, but it is a footgun as the code evolves.
The daemon already exposes `pub(crate) async fn execute_request(request: DaemonRequest, daemon: SharedDaemon)`.
**Fix**: Replace `send_daemon_request` in the HTTP API with a direct call to `daemon::execute_request`.
The refactor is mechanical:
```rust
// In execute_json and execute_json_value, instead of:
match send_daemon_request(daemon, &request).await { ... }
// Use:
let (response, _stop) = daemon::execute_request(request, daemon).await;
from_daemon_response::<T>(response)
```
Then delete `send_daemon_request` entirely. This eliminates ~60 lines of socket boilerplate from the
HTTP path, removes a class of failure modes, and eliminates the latent deadlock risk.
---
### 5. DOCUMENTATION STALENESS — PROJECT_MAP.md out of date
File: `PROJECT_MAP.md`
The "What Has NOT Been Started" section still says:
```
- HTTP route handlers and request validation
```
This was completed in Milestone 5. Update that section to reflect the current state (Milestone 6 in
progress, remaining items are tracing log output, systemd unit generation, CI, cross-compile test,
and first binary release).
Also update the `## Project Status` block:
- Change "Phase: Milestone 5 in progress" to "Phase: Milestone 6 in progress"
- Update the description to reflect that the HTTP API is complete
---
### 6. CLEANUP — Accidental files in the archive / repo
The project archive contains two directories that must not be in the repo:
- `node-compile-cache/` — Node.js V8 bytecode cache, clearly from the developer's working environment
- `phantomjs/` — A 23 MB PhantomJS binary distribution
Neither is referenced by the Rust project in any way. Confirm these are excluded in `.gitignore` and
remove them from any committed state if they were accidentally committed. The `.gitignore` should
include at minimum:
```
node-compile-cache/
phantomjs/
```
---
### 7. MINOR — `get_value` allocates a full BTreeMap to look up one key
File: `src/daemon/config.rs`, `get_value` method
```rust
pub fn get_value(&self, key: &str) -> Option<String> {
self.entries().remove(key) // allocates entire map just to look up one key
}
```
This allocates and populates a full `BTreeMap` (15 entries) just to retrieve one value.
**Fix**: Inline the lookup directly in `get_value` using a `match` on the key, identical to the
pattern already used in `set_value`. This is a micro-optimization but it also eliminates the
inconsistency where `entries()` and `set_value` are two separate parallel tables that must stay in
sync. Alternatively, make `set_value` call `entries()` for validation but that's heavier. The
simplest correct fix is to re-express `get_value` as a `match` block mirroring `set_value`.
---
### 8. MINOR — `roku_key_paths` uses mixed return styles
File: `src/adapters/roku/mod.rs`, function `roku_key_paths`
The function has two code paths:
1. The first ~25 arms `return Ok(vec!["SomeKey".to_string()])` inline
2. The last 5 arms fall through to a `let path = match { "some-string" }` followed by
`Err(TvError::InvalidKey(path.to_string()))`
This is intentional (the last 5 keys have no known Roku mapping), but the pattern is confusing:
those 5 arms look like they map to valid paths but actually the entire bottom branch returns `Err`.
**Fix**: Consolidate to make the intent obvious. The simplest approach: keep the two-style structure
but add a comment before the fall-through block making it explicit that these keys are
**intentionally unsupported** and the string values are used only for the error message:
```rust
// The following keys have no documented Roku ECP equivalent.
// The string values are used only to populate the InvalidKey error.
TvKey::Stop => "stop",
TvKey::Skip => "skip",
TvKey::Power => "power",
TvKey::PowerOn => "power-on",
TvKey::Options => "options",
```
No logic change required — just a documentation fix to prevent future maintainers from trying to
"fix" this by making it return Ok.
---
### 9. MINOR — `RefreshAppsBody` optional body field can use `unwrap_or_default`
File: `src/api/mod.rs`
```rust
clear: body.map(|value| value.clear).unwrap_or(false),
```
`bool` implements `Default` (defaults to `false`), so this can be simplified to:
```rust
clear: body.map_or(false, |value| value.clear),
```
Or better: make `clear` default-able in the body by giving it `#[serde(default)]` (it already has
that) and unwrap the optional body using `unwrap_or_default()` on the struct. Either is fine.
This is cosmetic.
---
### 10. NOTE — `edition = "2024"` in Cargo.toml
File: `Cargo.toml`
The crate uses `edition = "2024"`. Rust 2024 edition stabilized in Rust 1.85 (Feb 2025).
This is fine and intentional, but it explains the `if clear && let Err(...)` syntax in `daemon/mod.rs`
which is a 2024-edition `let chains` feature. No change needed — just confirm the CI target
(when it lands) pins to `rustup toolchain` >= 1.85.
```
Let me check the remaining files I haven't read yet — the CLI's full body and the cache/state files.Good — now I have seen everything. Let me produce the complete, final agent instructions.Now I have a complete picture. Here are the full, final agent instructions:
---
## Complete Agent Instructions for tvctl
```
You are working on the `tvctl` Rust project. Read PROJECT_MAP.md and ROADMAP.md first.
Do NOT rewrite files wholesale. Make targeted, surgical str_replace changes only.
Do NOT touch any code not explicitly listed in the tasks below.
After all changes, run `cargo clippy` and `cargo test` and fix any new warnings before finishing.
---
## VERIFIED PROJECT STATUS
Milestones 15 are complete. Milestone 6 is partially done. The following Milestone 6 items
remain open per ROADMAP.md:
- `tvctl daemon install` generating a correct systemd unit ← DONE (render_systemd_unit exists)
- Log output via `tracing` (respects `log_level` config) ← STILL OPEN
- Cross-compile test ← STILL OPEN
- GitHub Actions CI ← STILL OPEN
- First binary release ← STILL OPEN
**The PROJECT_MAP.md is stale.** Update it as part of these tasks (see Task 9).
---
## TASK 1 — CRITICAL BUG: Eliminate `format_tv_key` — it is a duplicate of `parse_normalized_tv_key` in reverse
**File:** `src/daemon/mod.rs`
`format_tv_key` is a 33-arm match over `TvKey` that reconstructs the kebab-case name for each key.
`TvKey` is already decorated with `#[serde(rename_all = "kebab-case")]`, so serde already owns
this mapping. `format_tv_key` will silently drift if any variant is added or renamed. There is no
compiler enforcement.
**Step 1:** In `src/adapters/mod.rs`, implement `std::fmt::Display` for `TvKey` using serde's
rename output as the source of truth:
```rust
impl std::fmt::Display for TvKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// Delegate to serde's kebab-case serialization so the display name
// is always consistent with what the CLI and API accept as input.
let name = serde_json::to_value(self)
.ok()
.and_then(|v| v.as_str().map(str::to_string))
.unwrap_or_else(|| format!("{self:?}"));
f.write_str(&name)
}
}
```
Place this impl block directly after the `TvKey` enum definition.
**Step 2:** In `src/daemon/mod.rs`, find the single call site of `format_tv_key`:
```rust
let detail = format!("Sent key '{}' to {}.", format_tv_key(&key), device.name);
```
Replace it with:
```rust
let detail = format!("Sent key '{key}' to {}.", device.name);
```
(This works because `TvKey` now implements `Display`.)
**Step 3:** Delete the entire `format_tv_key` function from `src/daemon/mod.rs`.
---
## TASK 2 — BUG: `DeviceRegistry::remove` duplicates the match logic of `DeviceRegistry::find`
**File:** `src/daemon/registry.rs`
`find` (line ~84) and `matches_target` (line ~322) contain identical UUID + case-insensitive name
matching logic. They will silently diverge if matching rules change.
**Step 1:** Replace the body of `remove` so it delegates to `find`:
Current:
```rust
pub fn remove(&mut self, target: &str) -> Option<Device> {
let index = self
.devices
.iter()
.position(|device| matches_target(device, target))?;
let removed = self.devices.remove(index);
self.ensure_default();
Some(removed)
}
```
Replace with:
```rust
pub fn remove(&mut self, target: &str) -> Option<Device> {
let id = self.find(target)?.id;
let index = self.devices.iter().position(|d| d.id == id)?;
let removed = self.devices.remove(index);
self.ensure_default();
Some(removed)
}
```
**Step 2:** Delete the `matches_target` free function at the bottom of the file entirely.
---
## TASK 3 — BUG: `parse_bool` is a zero-value wrapper
**File:** `src/daemon/config.rs`
```rust
fn parse_bool(key: &str, value: &str) -> anyhow::Result<bool> {
parse_value(key, value)
}
```
This function does nothing that `parse_value::<bool>` does not already do.
**Step 1:** Delete the `parse_bool` function.
**Step 2:** Replace its three call sites:
- `parse_bool(key, value)?` → `parse_value(key, value)?`
The Rust type inference will resolve `parse_value` to `bool` from the assignment context in each case.
Confirm that `cargo build` passes after this change.
---
## TASK 4 — ARCHITECTURE BUG: HTTP API routes through the Unix socket when it could call directly
**File:** `src/api/mod.rs`
The HTTP API server runs **inside the same process** as the daemon. It holds `SharedDaemon`. Despite
this, every HTTP handler calls `send_daemon_request`, which opens a new Unix socket connection back
to the same daemon process, serializes the request to JSON, sends it over the socket, and deserializes
the response back.
Problems:
1. Unnecessary OS round-trip on every HTTP request.
2. A class of failures (`daemon_socket_unreachable`) that can never legitimately occur for in-process calls.
3. Latent deadlock risk: handler acquires the Mutex to read the socket path, releases it, then the
request re-enters via the socket handler and tries to acquire the Mutex again. Works today only
because `handle_connection` holds no lock during execution, but this is not enforced.
`daemon::execute_request` is already `pub(crate)` and accepts `(DaemonRequest, SharedDaemon)`.
**Step 1:** Replace the two implementations of `execute_json` and `execute_json_value` with versions
that call `daemon::execute_request` directly:
```rust
async fn execute_json<T>(daemon: SharedDaemon, request: DaemonRequest) -> Response
where
T: Serialize + DeserializeOwned,
{
let (response, _) = crate::daemon::execute_request(request, daemon).await;
from_daemon_response::<T>(response)
}
async fn execute_json_value(daemon: SharedDaemon, request: DaemonRequest) -> Response {
let (response, _) = crate::daemon::execute_request(request, daemon).await;
from_daemon_response::<Value>(response)
}
```
**Step 2:** Delete the entire `send_daemon_request` async function (~65 lines).
**Step 3:** Remove the now-unused imports from `src/api/mod.rs`:
- `tokio::io::{AsyncReadExt, AsyncWriteExt}`
- `tokio::net::UnixStream`
Confirm `cargo build` passes after this change.
---
## TASK 5 — BUG: `PowerState` rendered with `{:?}` in human-readable output
**File:** `src/cli/mod.rs`, function `render_state`
```rust
format!("Power: {:?}", state.power),
```
`{:?}` prints `On`, `Off`, or `Unknown` (Rust variant names). This is fine but inconsistent with
the rest of the output, which is lowercase prose. Worse, it will break if `PowerState` is ever
renamed or repr-changed.
`PowerState` is already `#[serde(rename_all = "snake_case")]`, so it serializes to `"on"`, `"off"`,
`"unknown"`.
**Fix:** Implement `std::fmt::Display` for `PowerState` in `src/adapters/mod.rs`, mirroring the
serde rename:
```rust
impl std::fmt::Display for PowerState {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
PowerState::On => f.write_str("on"),
PowerState::Off => f.write_str("off"),
PowerState::Unknown => f.write_str("unknown"),
}
}
}
```
Then in `render_state`, change:
```rust
format!("Power: {:?}", state.power),
```
to:
```rust
format!("Power: {}", state.power),
```
---
## TASK 6 — BUG: Secret redaction logic is duplicated across three locations
**Files:** `src/cli/mod.rs`, `src/api/mod.rs`
The password redaction pattern appears in three places:
1. `src/cli/mod.rs` — `redact_config_value` (checks `is_secret_config_key`) — used in `config list`
and `config get`
2. `src/cli/mod.rs` — `sanitize_config_reload_result` (manually mutates `roku_password`) — used in
`config reload`
3. `src/api/mod.rs` — `get_config` (manually mutates `roku_password`) — used in `GET /v1/config`
These three will drift if new secret keys are added to the config.
**Fix:**
**Step 1:** Add a `redact_secrets` method to `TvctlConfig` in `src/daemon/config.rs`:
```rust
impl TvctlConfig {
/// Return a copy of this config with sensitive fields replaced by "<redacted>".
pub fn redacted(&self) -> Self {
let mut copy = self.clone();
if !copy.dev.roku_password.is_empty() {
copy.dev.roku_password = "<redacted>".to_string();
}
copy
}
}
```
**Step 2:** In `src/api/mod.rs`, replace the manual redaction in `get_config`:
Current:
```rust
Ok(mut config) => {
if !config.dev.roku_password.is_empty() {
config.dev.roku_password = "<redacted>".to_string();
}
api_success(StatusCode::OK, config)
}
```
Replace with:
```rust
Ok(config) => api_success(StatusCode::OK, config.redacted()),
```
**Step 3:** In `src/cli/mod.rs`, replace `sanitize_config_reload_result`:
Current:
```rust
fn sanitize_config_reload_result(mut result: ConfigReloadResult) -> ConfigReloadResult {
if !result.config.dev.roku_password.is_empty() {
result.config.dev.roku_password = "<redacted>".to_string();
}
result
}
```
Replace with:
```rust
fn sanitize_config_reload_result(mut result: ConfigReloadResult) -> ConfigReloadResult {
result.config = result.config.redacted();
result
}
```
The existing `redact_config_value` / `is_secret_config_key` pair in the CLI can remain as-is for now
since it serves a different path (key-by-key display), but the two manual inline redactions are gone.
---
## TASK 7 — MINOR: `get_value` allocates a full BTreeMap to look up one key
**File:** `src/daemon/config.rs`
```rust
pub fn get_value(&self, key: &str) -> Option<String> {
self.entries().remove(key)
}
```
This allocates and populates a 15-entry `BTreeMap` to look up a single value. `set_value` already
has a `match` on the key — the same pattern should be used here.
**Fix:** Replace `get_value` with a match block that mirrors the structure of `set_value`:
```rust
pub fn get_value(&self, key: &str) -> Option<String> {
match key {
"daemon.socket" => Some(self.daemon.socket.clone()),
"daemon.http_enabled" => Some(self.daemon.http_enabled.to_string()),
"daemon.http_port" => Some(self.daemon.http_port.to_string()),
"daemon.http_host" => Some(self.daemon.http_host.clone()),
"daemon.log_level" => Some(self.daemon.log_level.clone()),
"discovery.auto_discover" => Some(self.discovery.auto_discover.to_string()),
"discovery.interval_secs" => Some(self.discovery.interval_secs.to_string()),
"discovery.timeout_secs" => Some(self.discovery.timeout_secs.to_string()),
"devices.default" => Some(self.devices.default.clone()),
"remote.roku_key_mode" => Some(self.remote.roku_key_mode.clone()),
"remote.roku_press_duration_ms" => Some(self.remote.roku_press_duration_ms.to_string()),
"dev.enabled" => Some(self.dev.enabled.to_string()),
"dev.roku_username" => Some(self.dev.roku_username.clone()),
"dev.roku_password" => Some(self.dev.roku_password.clone()),
_ => None,
}
}
```
This eliminates the allocation and keeps `get_value` and `set_value` as a symmetric pair that the
compiler will force you to update together.
---
## TASK 8 — MINOR: `roku_key_paths` unsupported-key arms need a clarifying comment
**File:** `src/adapters/roku/mod.rs`, function `roku_key_paths`
The first ~25 arms of `roku_key_paths` `return Ok(vec!["..."])`. The last 5 arms fall through to
`let path = match { ... }` followed by `Err(TvError::InvalidKey(...))`. This is intentional but
looks like the strings in the last 5 arms are ECP key paths when they are actually just error
message content.
**Fix:** Add a comment before the fall-through block, no logic change:
Find this section in `roku_key_paths`:
```rust
TvKey::Stop => "stop",
TvKey::Skip => "skip",
TvKey::Power => "power",
TvKey::PowerOn => "power-on",
TvKey::Options => "options",
};
Err(TvError::InvalidKey(path.to_string()))
```
Replace with:
```rust
// The following keys have no documented Roku ECP equivalent.
// These strings populate the InvalidKey error message only — they are NOT ECP paths.
TvKey::Stop => "stop",
TvKey::Skip => "skip",
TvKey::Power => "power",
TvKey::PowerOn => "power-on",
TvKey::Options => "options",
};
Err(TvError::InvalidKey(path.to_string()))
```
---
## TASK 9 — Update PROJECT_MAP.md and clean up the archive
**Step 1:** In `PROJECT_MAP.md`:
Under `## Project Status`, change:
```
**Phase:** Milestone 5 in progress. ...
```
to:
```
**Phase:** Milestone 6 in progress. Milestones 15 are complete. The HTTP API is live. Remaining
Milestone 6 work: tracing log level enforcement, cross-compile test, GitHub Actions CI, and first
binary release.
**Platform v1:** Roku only (via ECP HTTP API)
**Language:** Rust
**Crate type:** Binary (single binary distribution target)
```
Under `## What Has NOT Been Started`, remove the line:
```
- HTTP route handlers and request validation
```
Replace the entire section with what actually remains:
```
## What Has NOT Been Started
- Log level enforcement via `tracing` (config key `daemon.log_level` is parsed but the subscriber
filter is not updated at runtime after config reload)
- CI/CD configuration (GitHub Actions)
- Cross-compile targets (x86_64 + aarch64)
- First binary release
```
**Step 2:** Confirm that `node-compile-cache/` and `phantomjs/` are listed in `.gitignore`.
If `.gitignore` does not exist, create it at the project root with at minimum:
```
/target
node-compile-cache/
phantomjs/
*.lock
```
Do not add `Cargo.lock` to gitignore (binary crates should commit their lockfile).
The correct ignore for Cargo lockfile in a binary crate is to NOT ignore it. Keep `Cargo.lock` tracked.
---
## TASK 10 — NOTE: `AppInfo.id` and `AppInfo.platform_id` are always identical for Roku (no fix needed, but document)
In `src/adapters/roku/mod.rs`, every `AppInfo` constructed in `parse_apps` and `parse_active_app`
sets both `id` and `platform_id` to the same value:
```rust
AppInfo {
id: platform_id.to_string(),
name: name.to_string(),
version: ...,
platform_id: platform_id.to_string(),
}
```
The design intent (per `src/adapters/mod.rs` comments) is that `id` is a "tvctl-normalized app
identifier" and `platform_id` is the "raw platform-specific app identifier." For Roku these are
the same. For a future platform (e.g., Google TV via ADB) they could differ.
**No code change needed.** Add a comment in the Roku `parse_apps` function to document the intent:
```rust
Some(AppInfo {
// For Roku, the tvctl-normalized id and the raw ECP channel id are the same value.
// Future platforms may produce different normalized and platform-specific identifiers.
id: platform_id.to_string(),
name: name.to_string(),
version: node.attribute("version").map(ToString::to_string),
platform_id: platform_id.to_string(),
})
```
Apply the same comment in `parse_active_app`.
---
## VERIFICATION CHECKLIST
After completing all tasks:
1. `cargo build` — must produce zero errors
2. `cargo clippy -- -D warnings` — must produce zero warnings
3. `cargo test` — all tests must pass
4. Manually verify that Task 4 (API direct call) did not break the HTTP integration path by
confirming `execute_request` in `daemon/mod.rs` is `pub(crate)` (it already is)
5. Verify PROJECT_MAP.md and ROADMAP.md are consistent with each other after Task 9 edits
```
+11
View File
@@ -5,6 +5,8 @@
This file captures the v1 surface promised by the design bundle so the This file captures the v1 surface promised by the design bundle so the
repository has a dedicated API spec from day one. repository has a dedicated API spec from day one.
For browser-based tools, CORS is opt-in via daemon config.
## Base URL ## Base URL
`http://127.0.0.1:7272/v1` `http://127.0.0.1:7272/v1`
@@ -57,4 +59,13 @@ Error responses:
- `error.code` values are stable machine contracts. - `error.code` values are stable machine contracts.
- `error.hint` is optional but recommended for user-actionable failures. - `error.hint` is optional but recommended for user-actionable failures.
- Devices may be addressed by UUID or friendly name. - Devices may be addressed by UUID or friendly name.
- Path targets must be URL-encoded by clients (for example `Living Room TV` -> `Living%20Room%20TV`).
- User-supplied friendly names are trimmed; `/` and `%` are rejected (`invalid_device_name`).
- Platform-specific field names must not leak past the adapter layer. - Platform-specific field names must not leak past the adapter layer.
- Browser clients require `daemon.cors_enabled = true` and explicit `daemon.cors_allowed_origins`.
## Dashboard Example
An endpoint coverage dashboard is included at:
`examples/http-dashboard/`
+46
View File
@@ -0,0 +1,46 @@
# tvctl HTTP API Dashboard Example
This is a static browser dashboard for manual testing of every `tvctl` HTTP API endpoint.
## 1. Enable API + CORS
The API is already enabled by default. CORS is not.
Enable CORS for local browser usage:
```bash
tvctl config set daemon.cors_enabled true
tvctl config set daemon.cors_allowed_origins "http://127.0.0.1:8080,http://localhost:8080"
tvctl config reload
```
## 2. Serve the dashboard files
From the repository root:
```bash
python3 -m http.server 8080 -d examples/http-dashboard
```
Then open:
`http://127.0.0.1:8080`
Do not use the `http://0.0.0.0:8080` URL shown by Python's server output as the browser URL.
Use `http://127.0.0.1:8080` or `http://localhost:8080` so the request origin matches your
configured CORS allowlist.
## 3. Test flow
1. `GET /daemon/status`
2. `GET /devices`
3. `POST /devices/discover`
4. Select a target device and test device/apps/remote/dev sections
5. Use config section for `GET /config`, `PATCH /config`, `POST /config/reload`
## Notes
- Destructive/mutating actions prompt for confirmation.
- `dev/install` uses multipart upload (`archive` field) as required by the API.
- Device targets are URL-encoded automatically before calling `/v1/devices/{id}/...` routes.
- Requests and responses are shown in the response panel for debugging.
+360
View File
@@ -0,0 +1,360 @@
const baseUrlInput = document.getElementById("baseUrl");
const targetOverrideInput = document.getElementById("targetOverride");
const deviceSelect = document.getElementById("deviceSelect");
const requestLog = document.getElementById("requestLog");
const responseView = document.getElementById("responseView");
const appLaunchValue = document.getElementById("appLaunchValue");
const refreshClear = document.getElementById("refreshClear");
const singleKey = document.getElementById("singleKey");
const sequenceKeys = document.getElementById("sequenceKeys");
const sequenceDelay = document.getElementById("sequenceDelay");
const devZip = document.getElementById("devZip");
const configKey = document.getElementById("configKey");
const configValue = document.getElementById("configValue");
function getBaseUrl() {
return baseUrlInput.value.replace(/\/+$/, "");
}
function getDeviceTarget() {
const override = targetOverrideInput.value.trim();
if (override.length > 0) return override;
const selected = deviceSelect.value.trim();
if (selected.length > 0) return selected;
throw new Error("Select a device or enter a manual override target first.");
}
function setResponse(value) {
responseView.value = value;
}
function appendLog(line) {
const timestamp = new Date().toISOString();
const current = requestLog.value || "";
requestLog.value = `[${timestamp}] ${line}\n${current}`.trim();
}
function safeParseJsonInput(input) {
try {
return JSON.parse(input);
} catch {
return input;
}
}
async function callJson(path, method = "GET", body = null, extraHeaders = {}) {
const url = `${getBaseUrl()}${path}`;
const headers = { ...extraHeaders };
const init = { method, headers };
if (body !== null) {
headers["Content-Type"] = "application/json";
init.body = JSON.stringify(body);
}
appendLog(`${method} ${url}`);
let response;
try {
response = await fetch(url, init);
} catch (error) {
const origin = window.location.origin;
setResponse(
JSON.stringify(
{
request: { method, url, body, origin },
error: {
kind: "network_or_cors",
message: String(error),
hint:
"If tvctl API is running, this is likely CORS. Enable daemon.cors_enabled and add this origin to daemon.cors_allowed_origins.",
},
},
null,
2
)
);
throw error;
}
const raw = await response.text();
let parsed = null;
try {
parsed = JSON.parse(raw);
} catch {
parsed = { non_json_body: raw };
}
setResponse(
JSON.stringify(
{
request: { method, url, body },
response: {
status: response.status,
ok: response.ok,
headers: Object.fromEntries(response.headers.entries()),
body: parsed,
},
},
null,
2
)
);
if (!response.ok) {
throw new Error(`${method} ${path} failed with status ${response.status}`);
}
return parsed;
}
async function callMultipart(path, file) {
const target = getDeviceTarget();
const url = `${getBaseUrl()}${path.replace("{id}", encodeURIComponent(target))}`;
const form = new FormData();
form.append("archive", file);
appendLog(`POST ${url} (multipart archive=${file.name})`);
let response;
try {
response = await fetch(url, { method: "POST", body: form });
} catch (error) {
const origin = window.location.origin;
setResponse(
JSON.stringify(
{
request: {
method: "POST",
url,
body: `multipart archive=${file.name}`,
origin,
},
error: {
kind: "network_or_cors",
message: String(error),
hint:
"If tvctl API is running, this is likely CORS. Enable daemon.cors_enabled and add this origin to daemon.cors_allowed_origins.",
},
},
null,
2
)
);
throw error;
}
const raw = await response.text();
let parsed = null;
try {
parsed = JSON.parse(raw);
} catch {
parsed = { non_json_body: raw };
}
setResponse(
JSON.stringify(
{
request: {
method: "POST",
url,
body: `multipart archive=${file.name}`,
},
response: {
status: response.status,
ok: response.ok,
headers: Object.fromEntries(response.headers.entries()),
body: parsed,
},
},
null,
2
)
);
if (!response.ok) {
throw new Error(`POST ${path} failed with status ${response.status}`);
}
}
async function runAction(action) {
try {
await action();
} catch (error) {
appendLog(`ERROR: ${error}`);
}
}
function requireConfirm(message) {
return window.confirm(message);
}
async function refreshDeviceList() {
const json = await callJson("/devices");
const devices = json?.data ?? [];
deviceSelect.innerHTML = "";
if (devices.length === 0) {
deviceSelect.innerHTML = "<option value=''>no devices</option>";
return;
}
for (const device of devices) {
const value = device.id || device.name;
const option = document.createElement("option");
option.value = value;
option.textContent = `${device.name} (${device.id})`;
if (device.is_default) {
option.selected = true;
}
deviceSelect.appendChild(option);
}
}
document.getElementById("pingBtn").addEventListener("click", () =>
runAction(async () => {
await callJson("/daemon/status");
})
);
document.getElementById("listDevicesBtn").addEventListener("click", () =>
runAction(async () => {
await refreshDeviceList();
})
);
document.getElementById("discoverBtn").addEventListener("click", () =>
runAction(async () => {
await callJson("/devices/discover", "POST");
await refreshDeviceList();
})
);
document.getElementById("getDeviceBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}`);
})
);
document.getElementById("deleteDeviceBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
if (!requireConfirm(`Delete device '${target}'?`)) return;
await callJson(`/devices/${encodeURIComponent(target)}`, "DELETE");
await refreshDeviceList();
})
);
document.getElementById("getStateBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}/state`);
})
);
document.getElementById("listAppsBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}/apps`);
})
);
document.getElementById("launchAppBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
const app = appLaunchValue.value.trim();
if (!app) throw new Error("Launch app value is required.");
if (!requireConfirm(`Launch '${app}' on '${target}'?`)) return;
await callJson(`/devices/${encodeURIComponent(target)}/apps/launch`, "POST", { app });
})
);
document.getElementById("stopAppBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
if (!requireConfirm(`Stop active app on '${target}'?`)) return;
await callJson(`/devices/${encodeURIComponent(target)}/apps/stop`, "POST");
})
);
document.getElementById("refreshAppsBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}/apps/refresh`, "POST", {
clear: refreshClear.checked,
});
})
);
document.getElementById("sendKeyBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
const key = singleKey.value.trim();
if (!key) throw new Error("Single key is required.");
await callJson(`/devices/${encodeURIComponent(target)}/remote/key`, "POST", { key });
})
);
document.getElementById("sendSequenceBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
const keys = sequenceKeys.value
.split(",")
.map((item) => item.trim())
.filter(Boolean);
if (keys.length === 0) throw new Error("At least one sequence key is required.");
await callJson(`/devices/${encodeURIComponent(target)}/remote/sequence`, "POST", {
keys,
delay_ms: Number(sequenceDelay.value || "0"),
});
})
);
document.getElementById("devInstallBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
if (!devZip.files || devZip.files.length === 0) {
throw new Error("Select a zip file before installing.");
}
if (!requireConfirm(`Install selected dev zip to '${target}'?`)) return;
await callMultipart(`/devices/{id}/dev/install`, devZip.files[0]);
})
);
document.getElementById("devReloadBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}/dev/reload`, "POST");
})
);
document.getElementById("devLogsBtn").addEventListener("click", () =>
runAction(async () => {
const target = getDeviceTarget();
await callJson(`/devices/${encodeURIComponent(target)}/dev/logs`);
})
);
document.getElementById("getConfigBtn").addEventListener("click", () =>
runAction(async () => {
await callJson("/config");
})
);
document.getElementById("patchConfigBtn").addEventListener("click", () =>
runAction(async () => {
const key = configKey.value.trim();
if (!key) throw new Error("Config key is required.");
if (!requireConfirm(`Patch config key '${key}'?`)) return;
const value = safeParseJsonInput(configValue.value.trim());
await callJson("/config", "PATCH", { [key]: value });
})
);
document.getElementById("reloadConfigBtn").addEventListener("click", () =>
runAction(async () => {
if (!requireConfirm("Reload daemon config now?")) return;
await callJson("/config/reload", "POST");
})
);
window.addEventListener("error", (event) => {
appendLog(`ERROR: ${event.message}`);
});
window.addEventListener("unhandledrejection", (event) => {
appendLog(`ERROR: ${event.reason}`);
});
runAction(refreshDeviceList);
+135
View File
@@ -0,0 +1,135 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>tvctl HTTP API Dashboard</title>
<link rel="stylesheet" href="./styles.css">
</head>
<body>
<header>
<h1>tvctl HTTP API Dashboard</h1>
<p>Local manual test dashboard for every <code>/v1</code> endpoint.</p>
</header>
<main>
<section class="panel">
<h2>Connection</h2>
<label>
API Base URL
<input id="baseUrl" value="http://127.0.0.1:7272/v1" />
</label>
<div class="row">
<button id="pingBtn" type="button">GET /daemon/status</button>
<button id="listDevicesBtn" type="button">GET /devices</button>
<button id="discoverBtn" type="button">POST /devices/discover</button>
</div>
<p class="hint">Tip: if browser requests fail, enable CORS in tvctl config first.</p>
</section>
<section class="panel">
<h2>Device Target</h2>
<label>
Known Devices
<select id="deviceSelect">
<option value="">(none loaded)</option>
</select>
</label>
<label>
Manual device UUID or name override
<input id="targetOverride" placeholder="optional" />
</label>
<p class="hint">Device endpoint actions use override when set, otherwise selected device.</p>
</section>
<section class="panel">
<h2>Device Endpoints</h2>
<div class="row">
<button id="getDeviceBtn" type="button">GET /devices/{id}</button>
<button id="deleteDeviceBtn" class="danger" type="button">DELETE /devices/{id}</button>
<button id="getStateBtn" type="button">GET /devices/{id}/state</button>
</div>
</section>
<section class="panel">
<h2>Apps Endpoints</h2>
<label>
Launch app name or id
<input id="appLaunchValue" placeholder="jellyfin" />
</label>
<label>
Refresh clear cache first
<input id="refreshClear" type="checkbox" />
</label>
<div class="row">
<button id="listAppsBtn" type="button">GET /devices/{id}/apps</button>
<button id="launchAppBtn" class="warn" type="button">POST /devices/{id}/apps/launch</button>
<button id="stopAppBtn" class="warn" type="button">POST /devices/{id}/apps/stop</button>
<button id="refreshAppsBtn" type="button">POST /devices/{id}/apps/refresh</button>
</div>
</section>
<section class="panel">
<h2>Remote Endpoints</h2>
<label>
Single key
<input id="singleKey" placeholder="home" />
</label>
<label>
Sequence keys (comma separated)
<input id="sequenceKeys" placeholder="home,down,select" />
</label>
<label>
Sequence delay_ms
<input id="sequenceDelay" type="number" value="200" min="0" />
</label>
<div class="row">
<button id="sendKeyBtn" type="button">POST /devices/{id}/remote/key</button>
<button id="sendSequenceBtn" type="button">POST /devices/{id}/remote/sequence</button>
</div>
</section>
<section class="panel">
<h2>Dev Endpoints</h2>
<label>
Zip file for sideload
<input id="devZip" type="file" accept=".zip,application/zip" />
</label>
<div class="row">
<button id="devInstallBtn" class="warn" type="button">POST /devices/{id}/dev/install</button>
<button id="devReloadBtn" type="button">POST /devices/{id}/dev/reload</button>
<button id="devLogsBtn" type="button">GET /devices/{id}/dev/logs</button>
</div>
</section>
<section class="panel">
<h2>Config Endpoints</h2>
<label>
Config patch key
<input id="configKey" placeholder="daemon.http_port" />
</label>
<label>
Config patch value (JSON literal, e.g. 7272, true, "text")
<input id="configValue" placeholder="7272" />
</label>
<div class="row">
<button id="getConfigBtn" type="button">GET /config</button>
<button id="patchConfigBtn" class="danger" type="button">PATCH /config</button>
<button id="reloadConfigBtn" class="warn" type="button">POST /config/reload</button>
</div>
</section>
<section class="panel panel-wide">
<h2>Request Log</h2>
<textarea id="requestLog" readonly></textarea>
</section>
<section class="panel panel-wide">
<h2>Response</h2>
<textarea id="responseView" readonly></textarea>
</section>
</main>
<script src="./app.js"></script>
</body>
</html>
+120
View File
@@ -0,0 +1,120 @@
* {
box-sizing: border-box;
}
body {
margin: 0;
font-family: "IBM Plex Sans", "Segoe UI", sans-serif;
background: linear-gradient(180deg, #eef4ff 0%, #f8fbff 100%);
color: #1b2a41;
}
header {
padding: 1.2rem 1.4rem;
background: #0b3954;
color: #f3f7fb;
}
h1 {
margin: 0 0 0.4rem 0;
font-size: 1.4rem;
}
main {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(320px, 1fr));
gap: 0.8rem;
padding: 0.8rem;
}
.panel {
background: #ffffff;
border: 1px solid #c6d6eb;
border-radius: 10px;
padding: 0.8rem;
box-shadow: 0 2px 8px rgba(15, 35, 55, 0.06);
}
.panel-wide {
grid-column: 1 / -1;
}
.panel-wide:nth-last-of-type(2) {
order: 100;
}
.panel-wide:last-of-type {
order: 101;
}
.panel h2 {
margin-top: 0;
font-size: 1rem;
}
label {
display: block;
margin: 0.4rem 0;
font-size: 0.9rem;
}
input,
select,
button {
width: 100%;
margin-top: 0.25rem;
border: 1px solid #9db3cf;
border-radius: 6px;
font-size: 0.9rem;
}
input,
select {
padding: 0.45rem;
background: #fbfdff;
}
button {
padding: 0.55rem;
cursor: pointer;
background: #1d6fa5;
color: #ffffff;
font-weight: 600;
}
button.warn {
background: #af6d1d;
}
button.danger {
background: #a12626;
}
.row {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(160px, 1fr));
gap: 0.45rem;
margin-top: 0.5rem;
}
.hint {
font-size: 0.8rem;
color: #425b7a;
}
textarea[readonly] {
width: 100%;
min-height: 220px;
max-height: 70vh;
resize: both;
overflow: auto;
margin: 0;
padding: 0.6rem;
border: 1px solid #2f4b66;
border-radius: 6px;
background: #0f1720;
color: #d9ecff;
font-size: 0.8rem;
font-family: "IBM Plex Mono", "Cascadia Code", monospace;
white-space: pre;
}
+44 -4
View File
@@ -3,19 +3,20 @@ use std::collections::BTreeMap;
use axum::{ use axum::{
Json, Router, Json, Router,
extract::{Multipart, Path, State}, extract::{Multipart, Path, State},
http::StatusCode, http::{HeaderValue, Method, StatusCode},
response::{IntoResponse, Response}, response::{IntoResponse, Response},
routing::{get, post}, routing::{get, post},
}; };
use serde::{Deserialize, Serialize, de::DeserializeOwned}; use serde::{Deserialize, Serialize, de::DeserializeOwned};
use serde_json::Value; use serde_json::Value;
use tokio::fs; use tokio::fs;
use tower_http::cors::CorsLayer;
use crate::{ use crate::{
adapters::{Device, TvKey, parse_normalized_tv_key}, adapters::{Device, TvKey, parse_normalized_tv_key},
daemon::{ daemon::{
self, SharedDaemon, self, SharedDaemon,
config::TvctlConfig, config::{DaemonConfig, TvctlConfig},
ipc::{ ipc::{
AppListResult, ConfigReloadResult, DaemonRequest, DaemonResponse, DaemonStatus, AppListResult, ConfigReloadResult, DaemonRequest, DaemonResponse, DaemonStatus,
DevLogsResult, DiscoveryResult, StateResult, DevLogsResult, DiscoveryResult, StateResult,
@@ -25,7 +26,12 @@ use crate::{
/// Create the HTTP router for the tvctl API. /// Create the HTTP router for the tvctl API.
pub fn router(daemon: SharedDaemon) -> Router { pub fn router(daemon: SharedDaemon) -> Router {
Router::new() router_with_config(daemon, &DaemonConfig::default())
}
/// Create the HTTP router for the tvctl API using daemon config.
pub fn router_with_config(daemon: SharedDaemon, daemon_config: &DaemonConfig) -> Router {
let mut router = Router::new()
.route("/v1/devices", get(list_devices)) .route("/v1/devices", get(list_devices))
.route("/v1/devices/discover", post(discover_devices)) .route("/v1/devices/discover", post(discover_devices))
.route("/v1/devices/{id}", get(get_device).delete(delete_device)) .route("/v1/devices/{id}", get(get_device).delete(delete_device))
@@ -42,7 +48,14 @@ pub fn router(daemon: SharedDaemon) -> Router {
.route("/v1/daemon/status", get(daemon_status)) .route("/v1/daemon/status", get(daemon_status))
.route("/v1/config", get(get_config).patch(patch_config)) .route("/v1/config", get(get_config).patch(patch_config))
.route("/v1/config/reload", post(reload_config)) .route("/v1/config/reload", post(reload_config))
.with_state(daemon) .with_state(daemon);
if daemon_config.cors_enabled
&& let Some(cors_layer) = cors_layer_from_origins(&daemon_config.cors_allowed_origins)
{
router = router.layer(cors_layer);
}
router
} }
/// The standard success envelope for API responses. /// The standard success envelope for API responses.
@@ -445,6 +458,7 @@ fn status_for_error(code: &str) -> StatusCode {
| "invalid_key" | "invalid_key"
| "invalid_config_patch" | "invalid_config_patch"
| "invalid_config_value" | "invalid_config_value"
| "invalid_device_name"
| "missing_archive" | "missing_archive"
| "invalid_multipart" => StatusCode::BAD_REQUEST, | "invalid_multipart" => StatusCode::BAD_REQUEST,
_ => StatusCode::INTERNAL_SERVER_ERROR, _ => StatusCode::INTERNAL_SERVER_ERROR,
@@ -463,10 +477,28 @@ async fn execute_daemon_request(daemon: SharedDaemon, request: DaemonRequest) ->
response response
} }
fn cors_layer_from_origins(origins: &[String]) -> Option<CorsLayer> {
let parsed_origins: Vec<HeaderValue> = origins
.iter()
.filter_map(|origin| HeaderValue::from_str(origin).ok())
.collect();
if parsed_origins.is_empty() {
return None;
}
Some(
CorsLayer::new()
.allow_origin(parsed_origins)
.allow_methods([Method::GET, Method::POST, Method::PATCH, Method::DELETE])
.allow_headers([axum::http::header::CONTENT_TYPE]),
)
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::sync::Arc; use std::sync::Arc;
use axum::http::StatusCode;
use tokio::sync::Mutex; use tokio::sync::Mutex;
use super::execute_daemon_request; use super::execute_daemon_request;
@@ -521,4 +553,12 @@ mod tests {
.expect("shutdown should be rejected via HTTP"); .expect("shutdown should be rejected via HTTP");
assert_eq!(error.code, "invalid_http_request"); assert_eq!(error.code, "invalid_http_request");
} }
#[test]
fn invalid_device_name_maps_to_bad_request() {
assert_eq!(
super::status_for_error("invalid_device_name"),
StatusCode::BAD_REQUEST
);
}
} }
+28 -1
View File
@@ -1452,13 +1452,24 @@ fn default_marker(device: &Device) -> &'static str {
} }
fn render_systemd_unit(exe: &std::path::Path) -> String { fn render_systemd_unit(exe: &std::path::Path) -> String {
let escaped_exe = systemd_quote_exec_arg(exe);
format!( format!(
"[Unit]\nDescription=tvctl daemon\nAfter=network.target\n\n[Service]\nType=simple\nEnvironment={}=1\nExecStart={}\nRestart=on-failure\nRestartSec=2\n\n[Install]\nWantedBy=default.target\n", "[Unit]\nDescription=tvctl daemon\nAfter=network.target\n\n[Service]\nType=simple\nEnvironment={}=1\nExecStart={}\nRestart=on-failure\nRestartSec=2\n\n[Install]\nWantedBy=default.target\n",
daemon::INTERNAL_DAEMON_ENV, daemon::INTERNAL_DAEMON_ENV,
exe.display() escaped_exe
) )
} }
fn systemd_quote_exec_arg(path: &std::path::Path) -> String {
let escaped = path
.as_os_str()
.to_string_lossy()
.replace('\\', "\\\\")
.replace('"', "\\\"")
.replace('%', "%%");
format!("\"{escaped}\"")
}
fn is_secret_config_key(key: &str) -> bool { fn is_secret_config_key(key: &str) -> bool {
matches!(key, "dev.roku_password") matches!(key, "dev.roku_password")
} }
@@ -1503,4 +1514,20 @@ mod tests {
"help output should not include internal daemon entrypoint" "help output should not include internal daemon entrypoint"
); );
} }
#[test]
fn systemd_unit_uses_internal_daemon_env_and_expected_sections() {
let unit = render_systemd_unit(std::path::Path::new("/tmp/tvctl"));
assert!(unit.contains("[Unit]\nDescription=tvctl daemon"));
assert!(unit.contains("[Service]\nType=simple"));
assert!(unit.contains("Environment=TVCTL_INTERNAL_DAEMON=1"));
assert!(unit.contains("ExecStart=\"/tmp/tvctl\""));
assert!(unit.contains("[Install]\nWantedBy=default.target"));
}
#[test]
fn systemd_unit_escapes_special_characters_in_exec_path() {
let unit = render_systemd_unit(std::path::Path::new("/tmp/tv ctl/\"bin\"/%tvctl\\debug"));
assert!(unit.contains("ExecStart=\"/tmp/tv ctl/\\\"bin\\\"/%%tvctl\\\\debug\""));
}
} }
+32
View File
@@ -57,6 +57,11 @@ impl TvctlConfig {
("daemon.http_enabled", self.daemon.http_enabled.to_string()), ("daemon.http_enabled", self.daemon.http_enabled.to_string()),
("daemon.http_port", self.daemon.http_port.to_string()), ("daemon.http_port", self.daemon.http_port.to_string()),
("daemon.http_host", self.daemon.http_host.clone()), ("daemon.http_host", self.daemon.http_host.clone()),
("daemon.cors_enabled", self.daemon.cors_enabled.to_string()),
(
"daemon.cors_allowed_origins",
format_csv_list(&self.daemon.cors_allowed_origins),
),
("daemon.log_level", self.daemon.log_level.clone()), ("daemon.log_level", self.daemon.log_level.clone()),
( (
"discovery.auto_discover", "discovery.auto_discover",
@@ -89,6 +94,10 @@ impl TvctlConfig {
"daemon.http_enabled" => Some(self.daemon.http_enabled.to_string()), "daemon.http_enabled" => Some(self.daemon.http_enabled.to_string()),
"daemon.http_port" => Some(self.daemon.http_port.to_string()), "daemon.http_port" => Some(self.daemon.http_port.to_string()),
"daemon.http_host" => Some(self.daemon.http_host.clone()), "daemon.http_host" => Some(self.daemon.http_host.clone()),
"daemon.cors_enabled" => Some(self.daemon.cors_enabled.to_string()),
"daemon.cors_allowed_origins" => {
Some(format_csv_list(&self.daemon.cors_allowed_origins))
}
"daemon.log_level" => Some(self.daemon.log_level.clone()), "daemon.log_level" => Some(self.daemon.log_level.clone()),
"discovery.auto_discover" => Some(self.discovery.auto_discover.to_string()), "discovery.auto_discover" => Some(self.discovery.auto_discover.to_string()),
"discovery.interval_secs" => Some(self.discovery.interval_secs.to_string()), "discovery.interval_secs" => Some(self.discovery.interval_secs.to_string()),
@@ -110,6 +119,10 @@ impl TvctlConfig {
"daemon.http_enabled" => self.daemon.http_enabled = parse_value(key, value)?, "daemon.http_enabled" => self.daemon.http_enabled = parse_value(key, value)?,
"daemon.http_port" => self.daemon.http_port = parse_value(key, value)?, "daemon.http_port" => self.daemon.http_port = parse_value(key, value)?,
"daemon.http_host" => self.daemon.http_host = value.to_string(), "daemon.http_host" => self.daemon.http_host = value.to_string(),
"daemon.cors_enabled" => self.daemon.cors_enabled = parse_value(key, value)?,
"daemon.cors_allowed_origins" => {
self.daemon.cors_allowed_origins = parse_csv_list(value)
}
"daemon.log_level" => { "daemon.log_level" => {
validate_log_level(value)?; validate_log_level(value)?;
self.daemon.log_level = value.to_string(); self.daemon.log_level = value.to_string();
@@ -143,6 +156,10 @@ pub struct DaemonConfig {
pub http_port: u16, pub http_port: u16,
/// Loopback host or bind address. /// Loopback host or bind address.
pub http_host: String, pub http_host: String,
/// Whether browser CORS responses are enabled for the HTTP API.
pub cors_enabled: bool,
/// Comma-separated origin allowlist used when CORS is enabled.
pub cors_allowed_origins: Vec<String>,
/// Logging level. /// Logging level.
pub log_level: String, pub log_level: String,
} }
@@ -154,6 +171,8 @@ impl Default for DaemonConfig {
http_enabled: true, http_enabled: true,
http_port: 7272, http_port: 7272,
http_host: "127.0.0.1".to_string(), http_host: "127.0.0.1".to_string(),
cors_enabled: false,
cors_allowed_origins: Vec::new(),
log_level: "info".to_string(), log_level: "info".to_string(),
} }
} }
@@ -327,6 +346,19 @@ where
.map_err(|error| anyhow::anyhow!("invalid value '{value}' for {key}: {error}")) .map_err(|error| anyhow::anyhow!("invalid value '{value}' for {key}: {error}"))
} }
fn parse_csv_list(value: &str) -> Vec<String> {
value
.split(',')
.map(str::trim)
.filter(|item| !item.is_empty())
.map(ToString::to_string)
.collect()
}
fn format_csv_list(values: &[String]) -> String {
values.join(",")
}
fn validate_log_level(value: &str) -> anyhow::Result<()> { fn validate_log_level(value: &str) -> anyhow::Result<()> {
EnvFilter::try_new(value).map_err(|error| { EnvFilter::try_new(value).map_err(|error| {
anyhow::anyhow!("invalid value '{value}' for daemon.log_level: {error}") anyhow::anyhow!("invalid value '{value}' for daemon.log_level: {error}")
+117 -2
View File
@@ -189,12 +189,13 @@ async fn set_socket_permissions(path: &Path) -> anyhow::Result<()> {
async fn start_http_server_if_enabled( async fn start_http_server_if_enabled(
daemon: SharedDaemon, daemon: SharedDaemon,
) -> anyhow::Result<Option<JoinHandle<()>>> { ) -> anyhow::Result<Option<JoinHandle<()>>> {
let (enabled, host, port) = { let (enabled, host, port, daemon_config) = {
let guard = daemon.lock().await; let guard = daemon.lock().await;
( (
guard.config.daemon.http_enabled, guard.config.daemon.http_enabled,
guard.config.daemon.http_host.clone(), guard.config.daemon.http_host.clone(),
guard.config.daemon.http_port, guard.config.daemon.http_port,
guard.config.daemon.clone(),
) )
}; };
@@ -204,7 +205,7 @@ async fn start_http_server_if_enabled(
let address: SocketAddr = format!("{host}:{port}").parse()?; let address: SocketAddr = format!("{host}:{port}").parse()?;
let listener = tokio::net::TcpListener::bind(address).await?; let listener = tokio::net::TcpListener::bind(address).await?;
let app = api::router(daemon); let app = api::router_with_config(daemon, &daemon_config);
let task = tokio::spawn(async move { let task = tokio::spawn(async move {
if let Err(error) = axum::serve(listener, app).await { if let Err(error) = axum::serve(listener, app).await {
warn!("HTTP API server stopped: {error}"); warn!("HTTP API server stopped: {error}");
@@ -320,6 +321,10 @@ pub(crate) async fn execute_request(
port, port,
name, name,
} => { } => {
let name = match validate_friendly_name(name) {
Ok(name) => name,
Err(response) => return (response, false),
};
let mut guard = daemon.lock().await; let mut guard = daemon.lock().await;
match guard.adapters.probe_manual(&platform, address, port).await { match guard.adapters.probe_manual(&platform, address, port).await {
Ok(info) => { Ok(info) => {
@@ -886,6 +891,41 @@ fn dev_disabled_response() -> DaemonResponse {
) )
} }
fn validate_friendly_name(name: Option<String>) -> Result<Option<String>, DaemonResponse> {
let Some(name) = name else {
return Ok(None);
};
let trimmed = name.trim();
if trimmed.is_empty() {
return Err(DaemonResponse::error(
"invalid_device_name",
"Friendly name cannot be empty.",
Some(
"Choose a non-empty name, or omit --name to use the TV-reported name.".to_string(),
),
));
}
if trimmed.contains('/') || trimmed.contains('%') {
return Err(DaemonResponse::error(
"invalid_device_name",
format!("Friendly name '{trimmed}' contains unsupported characters."),
Some("Friendly names cannot include '/' or '%'. Use spaces, letters, numbers, '-' or '_'.".to_string()),
));
}
if trimmed.chars().any(char::is_control) {
return Err(DaemonResponse::error(
"invalid_device_name",
"Friendly name contains control characters.".to_string(),
Some("Remove tabs/newlines and retry.".to_string()),
));
}
Ok(Some(trimmed.to_string()))
}
fn resolve_target_device( fn resolve_target_device(
registry: &DeviceRegistry, registry: &DeviceRegistry,
target: Option<&str>, target: Option<&str>,
@@ -950,3 +990,78 @@ fn apply_runtime_config(daemon: &mut Daemon, config: TvctlConfig) -> anyhow::Res
daemon.config = config; daemon.config = config;
Ok(restart_required) Ok(restart_required)
} }
#[cfg(test)]
mod tests {
use std::sync::Arc;
use tokio::sync::Mutex;
use super::{Daemon, execute_request};
use crate::daemon::{
cache::AppCacheStore,
config::{RuntimePaths, TvctlConfig},
discovery::DiscoveryService,
ipc::DaemonRequest,
registry::{AdapterRegistry, DeviceRegistry},
state::StateCache,
};
async fn build_test_daemon() -> Arc<Mutex<Daemon>> {
let temp_dir = tempfile::tempdir().expect("temp dir should exist");
let root = temp_dir.path();
let data_dir = root.join("data");
std::fs::create_dir_all(&data_dir).expect("data dir should exist");
let paths = RuntimePaths {
config_file: root.join("config.toml"),
data_dir: data_dir.clone(),
devices_file: data_dir.join("devices.json"),
cache_dir: data_dir.join("cache"),
active_socket_file: data_dir.join("active_socket"),
socket_file: root.join("tvctl.sock"),
};
let config = TvctlConfig::default();
let adapters = AdapterRegistry::from_config(&config);
let registry = DeviceRegistry::load(paths.devices_file.clone())
.await
.expect("registry should load");
Arc::new(Mutex::new(Daemon {
config,
paths: paths.clone(),
registry,
app_cache: AppCacheStore::new(paths.cache_dir.clone()),
state_cache: StateCache::default(),
adapters: adapters.clone(),
discovery: DiscoveryService::new(adapters),
}))
}
#[tokio::test]
async fn add_device_rejects_unsafe_friendly_name_before_probe() {
let daemon = build_test_daemon().await;
let (response, should_stop) = execute_request(
DaemonRequest::AddDevice {
platform: "roku".to_string(),
address: "127.0.0.1".parse().expect("valid loopback ip"),
port: Some(8060),
name: Some("Living/Room".to_string()),
},
daemon,
)
.await;
assert!(!should_stop, "request should not stop daemon");
assert!(!response.ok, "invalid name should fail");
let error = response.error.expect("error payload should be present");
assert_eq!(error.code, "invalid_device_name");
}
#[test]
fn validate_friendly_name_allows_spaces_and_trims() {
let value = super::validate_friendly_name(Some(" Living Room TV ".to_string()))
.expect("valid friendly names should pass");
assert_eq!(value.as_deref(), Some("Living Room TV"));
}
}
+1
View File
@@ -0,0 +1 @@
- for the api, you can use uuid or friendly name, but often a friendly name includes spaces... how do we handle that?
+54 -2
View File
@@ -59,6 +59,8 @@ impl TestDaemon {
http_enabled: true, http_enabled: true,
http_port: port, http_port: port,
http_host: "127.0.0.1".to_string(), http_host: "127.0.0.1".to_string(),
cors_enabled: false,
cors_allowed_origins: Vec::new(),
log_level: "info".to_string(), log_level: "info".to_string(),
}, },
discovery: tvctl::daemon::config::DiscoveryConfig { discovery: tvctl::daemon::config::DiscoveryConfig {
@@ -153,6 +155,10 @@ impl Drop for TestDaemon {
impl InProcessApi { impl InProcessApi {
async fn start() -> Self { async fn start() -> Self {
Self::start_with_daemon_config(DaemonConfig::default()).await
}
async fn start_with_daemon_config(daemon_config: DaemonConfig) -> Self {
let temp_dir = tempfile::tempdir().expect("temp dir should exist"); let temp_dir = tempfile::tempdir().expect("temp dir should exist");
let root = temp_dir.path(); let root = temp_dir.path();
let config_home = root.join("config"); let config_home = root.join("config");
@@ -176,7 +182,9 @@ impl InProcessApi {
http_enabled: true, http_enabled: true,
http_port: port, http_port: port,
http_host: "127.0.0.1".to_string(), http_host: "127.0.0.1".to_string(),
log_level: "info".to_string(), cors_enabled: daemon_config.cors_enabled,
cors_allowed_origins: daemon_config.cors_allowed_origins,
log_level: daemon_config.log_level,
}, },
discovery: DiscoveryConfig { discovery: DiscoveryConfig {
auto_discover: false, auto_discover: false,
@@ -233,6 +241,7 @@ impl InProcessApi {
} else { } else {
registry.ensure_default(); registry.ensure_default();
} }
let daemon_http_config = config.daemon.clone();
let daemon: SharedDaemon = Arc::new(Mutex::new(Daemon { let daemon: SharedDaemon = Arc::new(Mutex::new(Daemon {
config, config,
paths: paths.clone(), paths: paths.clone(),
@@ -243,7 +252,7 @@ impl InProcessApi {
discovery: DiscoveryService::new(adapters), discovery: DiscoveryService::new(adapters),
})); }));
let app = tvctl::api::router(daemon); let app = tvctl::api::router_with_config(daemon, &daemon_http_config);
let server = tokio::spawn(async move { let server = tokio::spawn(async move {
axum::serve(listener, app) axum::serve(listener, app)
.await .await
@@ -397,3 +406,46 @@ async fn http_api_routes_requests_without_unix_socket_loopback() {
assert_eq!(devices_json["ok"], true); assert_eq!(devices_json["ok"], true);
assert_eq!(devices_json["data"][0]["id"], api.device.id.to_string()); assert_eq!(devices_json["data"][0]["id"], api.device.id.to_string());
} }
#[tokio::test]
async fn http_api_cors_is_disabled_by_default() {
let api = InProcessApi::start().await;
let client = Client::new();
let response = client
.get(format!("{}/devices", api.base_url))
.header("Origin", "http://127.0.0.1:8080")
.send()
.await
.expect("cors default response should arrive");
assert!(
response
.headers()
.get("access-control-allow-origin")
.is_none(),
"CORS headers should be absent by default"
);
}
#[tokio::test]
async fn http_api_cors_allows_configured_loopback_origin() {
let api = InProcessApi::start_with_daemon_config(DaemonConfig {
cors_enabled: true,
cors_allowed_origins: vec!["http://127.0.0.1:8080".to_string()],
..DaemonConfig::default()
})
.await;
let client = Client::new();
let response = client
.get(format!("{}/devices", api.base_url))
.header("Origin", "http://127.0.0.1:8080")
.send()
.await
.expect("cors allowed response should arrive");
assert_eq!(
response
.headers()
.get("access-control-allow-origin")
.and_then(|value| value.to_str().ok()),
Some("http://127.0.0.1:8080")
);
}