-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): authenticate LAN management API, redact cloud token #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,14 +3,55 @@ | |||||||||||||
| package main | ||||||||||||||
|
|
||||||||||||||
| import ( | ||||||||||||||
| "crypto/subtle" | ||||||||||||||
| "encoding/json" | ||||||||||||||
| "fmt" | ||||||||||||||
| "log" | ||||||||||||||
| "net/http" | ||||||||||||||
| "time" | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| func (a *Agent) startHTTPServer(port int) *http.Server { | ||||||||||||||
| // SEC-HARDENING (2026-09-14, HIGH): the LAN management API is reachable | ||||||||||||||
| // unauthenticated (binds all interfaces, runs as root). Reads stay open for | ||||||||||||||
| // the on-device dashboard, but (a) the cloud token is NEVER serialized and | ||||||||||||||
| // (b) every mutating route requires the device Bearer token. | ||||||||||||||
|
Comment on lines
+14
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨 Security: PUT /api/config response still serializes the raw cloud tokenThe PUT handler for /api/config (httpapi.go:169) encodes Reuse sanitizedConfig() for the PUT response instead of encoding the raw config.:
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎 |
||||||||||||||
|
|
||||||||||||||
| // sanitizedConfig returns the device config with the cloud token stripped. | ||||||||||||||
| // The token authenticates the device toward edge.wave.online; serving it on | ||||||||||||||
| // an unauthenticated LAN endpoint leaks it to any network peer (and to every | ||||||||||||||
| // browser that opens the dashboard, which polls /api/system). | ||||||||||||||
| func (a *Agent) sanitizedConfig() DeviceConfig { | ||||||||||||||
| a.mu.RLock() | ||||||||||||||
| defer a.mu.RUnlock() | ||||||||||||||
| c := a.config | ||||||||||||||
| c.CloudToken = "" | ||||||||||||||
| return c | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // requireDeviceAuth gates mutating routes on the device Bearer token. | ||||||||||||||
| // Fail-open ONLY while unprovisioned (no token exists yet, nothing to steal); | ||||||||||||||
| // once provisioned, missing/wrong credentials are rejected. | ||||||||||||||
| func (a *Agent) requireDeviceAuth(w http.ResponseWriter, r *http.Request) bool { | ||||||||||||||
| a.mu.RLock() | ||||||||||||||
| token := a.config.CloudToken | ||||||||||||||
| a.mu.RUnlock() | ||||||||||||||
| if token == "" { | ||||||||||||||
| return true | ||||||||||||||
|
Comment on lines
+38
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Prompt for AI agents
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+38
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+38
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Agentic Security Review
The comment frames this as a short unprovisioned window, but Impact: Any LAN peer can still invoke root-privileged module install/stop on devices that never received an out-of-band Reviewed by Cursor Security Reviewer for commit 74e06ea. Configure here. |
||||||||||||||
| got := r.Header.Get("Authorization") | ||||||||||||||
| if got == "" { | ||||||||||||||
| http.Error(w, "missing Authorization: Bearer <device-token>", 401) | ||||||||||||||
| return false | ||||||||||||||
| } | ||||||||||||||
| want := "Bearer " + token | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Because Prompt for AI agents |
||||||||||||||
| if subtle.ConstantTimeCompare([]byte(got), []byte(want)) != 1 { | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Authorization scheme matching is case-sensitive, so valid headers such as Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** httpapi.go
**Line:** 47:47
**Comment:**
*Api Mismatch: Authorization scheme matching is case-sensitive, so valid headers such as `bearer <device-token>` are rejected even though Bearer authentication schemes are case-insensitive.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+46
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||
| http.Error(w, "forbidden", 403) | ||||||||||||||
| return false | ||||||||||||||
| } | ||||||||||||||
| return true | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (a *Agent) buildMux() *http.ServeMux { | ||||||||||||||
| mux := http.NewServeMux() | ||||||||||||||
|
|
||||||||||||||
| // Web UI (embedded dashboard) | ||||||||||||||
|
|
@@ -26,10 +67,15 @@ func (a *Agent) startHTTPServer(port int) *http.Server { | |||||||||||||
| }) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| // System info | ||||||||||||||
| // System info (token-redacted; the dashboard polls this every 5s) | ||||||||||||||
| mux.HandleFunc("/api/system", func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||
| w.Header().Set("Content-Type", "application/json") | ||||||||||||||
| json.NewEncoder(w).Encode(a.SystemInfo()) | ||||||||||||||
| info := a.SystemInfo() | ||||||||||||||
| if dev, ok := info["device"].(DeviceConfig); ok { | ||||||||||||||
| dev.CloudToken = "" | ||||||||||||||
| info["device"] = dev | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+73
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||
| json.NewEncoder(w).Encode(info) | ||||||||||||||
| }) | ||||||||||||||
|
Comment on lines
71
to
79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Edge Case: SystemInfo() device-token redaction relies on a brittle type assertionThe /api/system handler (httpapi.go:73-78) redacts the token only if Was this helpful? React with 👍 / 👎 |
||||||||||||||
|
|
||||||||||||||
| // Module list | ||||||||||||||
|
|
@@ -46,12 +92,15 @@ func (a *Agent) startHTTPServer(port int) *http.Server { | |||||||||||||
| json.NewEncoder(w).Encode(a.HealthCheck()) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| // Module install | ||||||||||||||
| // Module install (privileged: executes install.sh as root — device auth required) | ||||||||||||||
| mux.HandleFunc("/api/modules/install", func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||
| if r.Method != http.MethodPost { | ||||||||||||||
| http.Error(w, "Method not allowed", 405) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| if !a.requireDeviceAuth(w, r) { | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| var req struct { | ||||||||||||||
| Name string `json:"name"` | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -67,12 +116,15 @@ func (a *Agent) startHTTPServer(port int) *http.Server { | |||||||||||||
| json.NewEncoder(w).Encode(map[string]string{"status": "installed", "module": req.Name}) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| // Module stop | ||||||||||||||
| // Module stop (privileged — device auth required) | ||||||||||||||
| mux.HandleFunc("/api/modules/stop", func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||
| if r.Method != http.MethodPost { | ||||||||||||||
| http.Error(w, "Method not allowed", 405) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| if !a.requireDeviceAuth(w, r) { | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
| var req struct { | ||||||||||||||
| Name string `json:"name"` | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -88,13 +140,16 @@ func (a *Agent) startHTTPServer(port int) *http.Server { | |||||||||||||
| json.NewEncoder(w).Encode(map[string]string{"status": "stopped", "module": req.Name}) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| // Device config | ||||||||||||||
| // Device config (reads: token-redacted; writes: device auth required) | ||||||||||||||
| mux.HandleFunc("/api/config", func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||
| switch r.Method { | ||||||||||||||
| case http.MethodGet: | ||||||||||||||
| w.Header().Set("Content-Type", "application/json") | ||||||||||||||
| json.NewEncoder(w).Encode(a.config) | ||||||||||||||
| json.NewEncoder(w).Encode(a.sanitizedConfig()) | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Config reads still race with writes During concurrent GET and PUT requests, Learn moreThe HTTP server handles requests concurrently. Example: One request changes Recommended fix: Guard the PUT mutation, timestamp update, persistence snapshot, and response snapshot with Was this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: After an authenticated Prompt for AI agents |
||||||||||||||
| case http.MethodPut: | ||||||||||||||
| if !a.requireDeviceAuth(w, r) { | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PUT config response leaks tokenMedium Severity A successful Reviewed by Cursor Bugbot for commit 74e06ea. Configure here. |
||||||||||||||
| var updates map[string]string | ||||||||||||||
| if err := json.NewDecoder(r.Body).Decode(&updates); err != nil { | ||||||||||||||
| http.Error(w, err.Error(), 400) | ||||||||||||||
|
|
@@ -146,9 +201,13 @@ func (a *Agent) startHTTPServer(port int) *http.Server { | |||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| return mux | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (a *Agent) startHTTPServer(port int) *http.Server { | ||||||||||||||
| server := &http.Server{ | ||||||||||||||
| Addr: fmt.Sprintf(":%d", port), | ||||||||||||||
| Handler: mux, | ||||||||||||||
| Handler: a.buildMux(), | ||||||||||||||
| ReadTimeout: 10 * time.Second, | ||||||||||||||
| WriteTimeout: 10 * time.Second, | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| package main | ||
|
|
||
| // SEC-HARDENING (2026-09-14, HIGH): the LAN management API bound all interfaces | ||
| // with zero auth (root daemon). Reads stay open for the on-device dashboard, | ||
| // but the cloud token is never serialized and every mutating route requires | ||
| // the device Bearer token. | ||
| import ( | ||
| "encoding/json" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func testAgent() *Agent { | ||
| return &Agent{ | ||
| config: DeviceConfig{ | ||
| DeviceID: "dev-1", | ||
| DeviceName: "test", | ||
| Platform: "linux", | ||
| Profile: "default", | ||
| CloudToken: "wvtok_test_secret", | ||
| }, | ||
| modules: map[string]*ModuleState{}, | ||
| } | ||
| } | ||
|
|
||
| func TestConfigRedactsCloudToken(t *testing.T) { | ||
| srv := httptest.NewServer(testAgent().buildMux()) | ||
| defer srv.Close() | ||
| res, err := http.Get(srv.URL + "/api/config") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| defer res.Body.Close() | ||
| var body map[string]any | ||
| if err := json.NewDecoder(res.Body).Decode(&body); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if tok, _ := body["cloud_token"].(string); tok != "" { | ||
| t.Fatalf("cloud token leaked in /api/config: %q", tok) | ||
| } | ||
| if body["device_id"] != "dev-1" { | ||
| t.Fatalf("expected device fields to survive redaction, got %v", body) | ||
| } | ||
| } | ||
|
|
||
| func TestMutatingRoutesRequireDeviceAuth(t *testing.T) { | ||
| srv := httptest.NewServer(testAgent().buildMux()) | ||
| defer srv.Close() | ||
|
|
||
| // no credentials → 401 | ||
| res, err := http.Post(srv.URL+"/api/modules/stop", "application/json", strings.NewReader(`{"name":"camera"}`)) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| res.Body.Close() | ||
| if res.StatusCode != 401 { | ||
| t.Fatalf("stop without auth = %d, want 401", res.StatusCode) | ||
| } | ||
|
|
||
| // wrong credentials → 403 (proves the handler reached auth, not the module) | ||
| req, _ := http.NewRequest(http.MethodPost, srv.URL+"/api/modules/stop", strings.NewReader(`{"name":"camera"}`)) | ||
| req.Header.Set("Authorization", "Bearer wrong") | ||
| res2, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| res2.Body.Close() | ||
| if res2.StatusCode != 403 { | ||
| t.Fatalf("stop with wrong auth = %d, want 403", res2.StatusCode) | ||
| } | ||
|
|
||
| // PUT config without auth → 401 | ||
| req3, _ := http.NewRequest(http.MethodPut, srv.URL+"/api/config", strings.NewReader(`{"device_name":"x"}`)) | ||
| res3, err := http.DefaultClient.Do(req3) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| res3.Body.Close() | ||
| if res3.StatusCode != 401 { | ||
| t.Fatalf("config PUT without auth = %d, want 401", res3.StatusCode) | ||
| } | ||
| } | ||
|
Comment on lines
+48
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win The authentication test covers only rejected credentials, so it would still pass if the new guard rejected every provisioned mutation. Add a request with the fixture's valid device Bearer token and assert it reaches each protected handler's post-auth behavior. 🧰 Tools🪛 ast-grep (0.45.3)[warning] 52-52: An outbound HTTP request ( (ssrf-http-get-tainted-url-go) 🪛 golangci-lint (2.13.2)[error] 57-57: Error return value of (errcheck) [error] 69-69: Error return value of (errcheck) [error] 53-53: net/http.Post must not be called. use net/http.NewRequestWithContext and (*net/http.Client).Do(*http.Request) (noctx) [error] 63-63: net/http.NewRequest must not be called. use net/http.NewRequestWithContext (noctx) [error] 75-75: net/http.NewRequest must not be called. use net/http.NewRequestWithContext (noctx) 🤖 Prompt for AI Agents |
||
|
|
||
| func TestReadsStayOpen(t *testing.T) { | ||
| srv := httptest.NewServer(testAgent().buildMux()) | ||
| defer srv.Close() | ||
| for _, p := range []string{"/health", "/api/system", "/api/modules", "/metrics"} { | ||
| res, err := http.Get(srv.URL + p) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| res.Body.Close() | ||
| if res.StatusCode != 200 { | ||
| t.Fatalf("GET %s = %d, want 200 (dashboard reads must stay open)", p, res.StatusCode) | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+28
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+86
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(httpapi_auth_test\.go|.*http.*\.go|main\.go)$' | head -80
printf '%s\n' '--- test ---'
test_file=$(git ls-files | rg '(^|/)httpapi_auth_test\.go$' | head -1)
if [ -z "$test_file" ]; then
echo 'httpapi_auth_test.go not found'
exit 0
fi
wc -l "$test_file"
sed -n '1,150p' "$test_file"
printf '%s\n' '--- route and handler references ---'
rg -n -C 5 'api/system|TestConfigRedactsCloudToken|cloud_token|CloudToken' --glob '*.go' .Repository: wave-av/wave-agent Length of output: 8619 Sensitive Data Exposure Reachability: External Assert that 🧰 Tools🪛 ast-grep (0.45.3)[warning] 89-89: An outbound HTTP request ( (ssrf-http-get-tainted-url-go) 🪛 golangci-lint (2.13.2)[error] 90-90: net/http.Get must not be called. use net/http.NewRequestWithContext and (*net/http.Client).Do(*http.Request) (noctx) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,7 +123,7 @@ func (a *Agent) saveConfig() error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| return os.WriteFile(filepath.Join(ConfigDir, "device.json"), data, 0644) | ||
| return os.WriteFile(filepath.Join(ConfigDir, "device.json"), data, 0600) // SEC-HARDENING (2026-09-14): token-bearing config is owner-only, never world-readable | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Existing config stays world-readableHigh Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 616740c. Configure here. |
||
| } | ||
|
|
||
| // safeNamePattern is the allowlist for caller-supplied module and profile | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟥 Config writes return the device token
A valid PUT returns
a.configwithCloudToken. API clients and intermediaries receive the reusable bearer credential.(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.