Add app template preview with ACL-protected defaults in New Server modal

[caffeinated92]

Summary

• add a read-only app template preview endpoint that returns raw template content and detected defaults
• show inline template preview in the New Server modal when an app template is selected
• prefill app host, port, and docker image from template defaults without overwriting manual edits
• protect template preview access with the app deployment ACL grant

Why

Creating app monitors from templates was missing an important feedback step: users could select a template, but they could not see what it contained before submitting. That made it harder to understand what host, port, image, and deployment settings a template would imply, and increased the chance of creating an app with the wrong assumptions.
This change improves the app creation flow by making template intent visible at selection time, while keeping the backend as the source of truth and preserving operator control over the final values.

Details

Backend

• add a new cluster-scoped endpoint to fetch raw app template content and parsed defaults
• return:
  • template content
  • app-host
  • app-port
  • prov-app-docker-img
• enforce cluster ACL checks and map the new route to GrantAppDeployment

Frontend

• fetch template preview when an app template is selected
• show raw TOML inline in the New Server modal
• auto-fill template defaults only when fields are still unset
• do not overwrite manually edited host or port
• detect dynamic template host values like {{...}} and prompt the user to set host manually instead

UX impact

This gives users immediate visibility into what a template does before creating an app monitor, which makes app initiation safer, more understandable, and easier to trust.

Security

Template preview is not public; it is protected by the same cluster ACL evaluation flow and explicitly requires the app deployment grant.

[claude]

PR Review: Add app template preview with ACL-protected defaults

Overall the feature is well-motivated and the UX improvement is clear. The ACL integration looks correct and the frontend state management (touched flags, dynamic-value detection) is sensible. That said, there are a few issues — one security-critical — that need addressing before merge.

---

Critical: Path Traversal in GetTemplateContent

File: cluster/cluster_app.go:699 / server/api_app.go:2396

The route uses {templateName:.*} which Gorilla mux allows to include slashes. A caller can supply a template name like ../../etc/passwd and the path construction in GetTemplateContent will resolve outside the templates directory:

localPath := filepath.Join(cluster.Conf.WorkingDir, ".templates", "apps", template+".toml")
// filepath.Join(".../.templates/apps", "../../etc/passwd.toml") → "...//etc/passwd.toml"

filepath.Join cleans .. components but does not constrain the result to the base directory. The same issue exists in loadTemplateFromShared which uses string concatenation ("app/deployments/" + templateName + ".toml"), which is even more permissive.

Fix: Validate the resolved path is under the expected base directory, or use filepath.Base(templateName) to strip any directory components before using it in path construction.

// In the handler, before calling GetTemplateContent:
templateName = filepath.Base(templateName)
if templateName == "." || templateName == "/" {
    http.Error(w, "Invalid template name", http.StatusBadRequest)
    return
}

Or in GetTemplateContent itself:

safe := filepath.Join(cluster.Conf.WorkingDir, ".templates", "apps")
localPath := filepath.Join(safe, filepath.Base(template)+".toml")
if !strings.HasPrefix(localPath, safe+string(os.PathSeparator)) {
    return nil, errors.New("invalid template path")
}

---

Bug: Writing response body after w.WriteHeader(200)

File: server/api_app.go:2426-2429

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
if err := json.NewEncoder(w).Encode(response); err != nil {
    http.Error(w, fmt.Sprintf("Error encoding response: %v", err), http.StatusInternalServerError)
    return
}

Once WriteHeader is called, the HTTP status code and headers are committed to the connection. The subsequent http.Error call will write text to an already-started 200 response body — the 500 status code is silently ignored. This is also logged as a Go warning: http: superfluous response.WriteHeader call.

Fix: Either log the encoding error and return, or structure it as other handlers do — buffer the response before writing headers:

w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(response); err != nil {
    // Headers not yet committed if Encode hasn't written anything — but safest to just log
    mycluster.LogModulePrintf(...)
    return
}

---

Minor: Cluster not found should return 404, not 500

File: server/api_app.go:2387

if mycluster == nil {
    http.Error(w, "No cluster", http.StatusInternalServerError)
    return
}

A missing cluster is a client error (bad cluster name), not a server error. Other read-only endpoints return 404 here; this handler should match. This is consistent across the new endpoint but not aligned with REST semantics.

---

Minor: Template content may expose sensitive values

GetTemplateContent returns the raw TOML bytes, which the handler passes directly to the frontend as "content". If a template contains secrets (passwords, API tokens) in its variables section, they would be returned verbatim to the browser.

Consider stripping sensitive fields before returning, or document clearly that templates must not contain secrets in plain text (only references).

---

Frontend: fetchTemplatePreview should use useCallback

File: NewServerModal.jsx

fetchTemplatePreview is redefined on every render, capturing stale values of hostTouched, portTouched, host, port, and dockerImage. While this is benign today (it's only called from event handlers), it should be wrapped in useCallback with a proper dependency array to be safe and to follow React best practices.

---

Missing tests

No tests were added for handlerMuxAppTemplatePreview. Given the ACL check and the path construction involved, at minimum:

• A test for a valid template fetch (mocked cluster with a known template)
• A test asserting that path traversal inputs return an error

---

Summary

Severity	Issue
🔴 Critical	Path traversal in GetTemplateContent via {templateName:.*} route
🟠 Bug	http.Error called after WriteHeader(200) — status 500 is dropped
🟡 Minor	mycluster == nil should return 404
🟡 Minor	Raw template content may contain sensitive values
🟢 Nit	fetchTemplatePreview missing useCallback
🟢 Nit	No test coverage for new endpoint

Requesting changes on the path traversal and the response-after-headers bug before merge. The rest can be addressed in follow-up issues if needed.