fix: Full repo review, bugs, dead code, docs, hardening

cmd/launch.go

@@ -1,11 +1,15 @@
 package cmd
 
-import "os/exec"
+import (
+	"os/exec"
+	"syscall"
+)
 
 // launchSubprocess starts a detached child process with the given binary
-// and arguments. The process inherits the current user's session (because
-// the service daemon already runs per-user).
+// and arguments. Setsid puts the child in its own session so parent signals
+// (e.g. SIGINT on daemon stop) don't kill in-flight UI subprocesses.
 func launchSubprocess(binary string, args []string) error {
 	cmd := exec.Command(binary, args...)
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 	return cmd.Start()
 }

cmd/motd.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/TsekNet/hermes/internal/app"
 	"github.com/spf13/cobra"
 )
 
@@ -75,10 +74,3 @@ func sanitize(s string) string {
 	}
 	return b.String()
 }
-
-// inboxEntryHeading extracts the heading from an InboxEntry.
-// This avoids importing the full app package just for the field name.
-func init() {
-	// Verify at compile time that app.InboxEntry has a Heading field.
-	var _ = app.InboxEntry{}.Heading
-}

cmd/notify.go

@@ -128,5 +128,8 @@ func writeTempConfig(cfg *config.NotificationConfig) (string, error) {
 		return "", fmt.Errorf("write temp config: %w", err)
 	}
 	f.Close()
+	// Make readable by all users so child processes in other sessions
+	// (launched via CreateProcessAsUser / setuid) can read the config.
+	os.Chmod(f.Name(), 0644)
 	return f.Name(), nil
 }

cmd/root.go

@@ -300,7 +300,7 @@ func resolveConfig(configFlag string, args []string) (*config.NotificationConfig
 		if trimmed == "" {
 			return nil, nil
 		}
-		return config.LoadJSON([]byte(trimmed))
+		return config.Load([]byte(trimmed))
 	}
 	return nil, nil
 }
@@ -313,12 +313,16 @@ func loadFromArg(arg string) (*config.NotificationConfig, error) {
 			return nil, fmt.Errorf("read %s: %w", arg, err)
 		}
 		deck.Infof("loaded config from file: %s", arg)
-		return config.LoadJSON(data)
+		return config.Load(data)
 	}
 
 	trimmed := strings.TrimSpace(arg)
-	if strings.HasPrefix(trimmed, "{") {
-		return config.LoadJSON([]byte(trimmed))
+	if strings.HasPrefix(trimmed, "{") || strings.Contains(trimmed, ":") {
+		cfg, err := config.Load([]byte(trimmed))
+		if err == nil {
+			return cfg, nil
+		}
+		return nil, fmt.Errorf("not a file or valid config: %s", arg)
 	}
 
 	return nil, fmt.Errorf("not a file or valid config: %s", arg)
@@ -339,7 +343,15 @@ func waitForDND(cfg *config.NotificationConfig) error {
 		}
 		return nil
 	default: // "respect"
+		const maxDNDWait = 24 * time.Hour
+		deadline := time.Now().Add(maxDNDWait)
 		for dnd.Active() {
+			if time.Now().After(deadline) {
+				deck.Warningf("notification: dnd=respect, max wait (%s) exceeded heading=%q", maxDNDWait, cfg.Heading)
+				fmt.Print("dnd_timeout")
+				os.Stdout.Sync()
+				os.Exit(int(exitcodes.Timeout))
+			}
 			deck.Infof("notification: dnd=respect, waiting 60s heading=%q", cfg.Heading)
 			time.Sleep(60 * time.Second)
 		}
@@ -406,14 +418,15 @@ func webview2DataPath() string {
 	return p
 }
 
-// prepareConfig applies defaults and locale resolution to a config.
+// prepareConfig applies defaults, locale resolution, and HTML escaping to a config.
 func prepareConfig(cfg *config.NotificationConfig) {
 	cfg.ApplyDefaults()
 	locale := flagLocale
 	if locale == "" {
 		locale = config.DetectLocale()
 	}
 	cfg.ApplyLocale(locale)
+	cfg.SanitizeText()
 }
 
 // respond prints the value to stdout and exits with the appropriate code.

cmd/sessionlaunch_windows.go

@@ -206,21 +206,27 @@ func quoteArg(s string) string {
 	if s == "" || strings.ContainsAny(s, ` "\`) {
 		var b strings.Builder
 		b.WriteByte('"')
-		nbs := 0
+		backslashes := 0
 		for i := 0; i < len(s); i++ {
 			switch s[i] {
 			case '\\':
-				nbs++
+				backslashes++
 			case '"':
-				b.WriteString(strings.Repeat(`\`, nbs+1))
-				nbs = 0
+				// Double backslashes before a quote, then escape the quote.
+				b.WriteString(strings.Repeat(`\`, backslashes*2+1))
+				backslashes = 0
 				b.WriteByte('"')
 			default:
-				nbs = 0
+				// Flush accumulated backslashes as-is (not before a quote).
+				if backslashes > 0 {
+					b.WriteString(strings.Repeat(`\`, backslashes))
+					backslashes = 0
+				}
 				b.WriteByte(s[i])
 			}
 		}
-		b.WriteString(strings.Repeat(`\`, nbs))
+		// Double trailing backslashes (they precede the closing quote).
+		b.WriteString(strings.Repeat(`\`, backslashes*2))
 		b.WriteByte('"')
 		return b.String()
 	}

docs/architecture.md

@@ -10,7 +10,7 @@ The notification UI is a web page, not a native dialog. HTML, CSS, and JavaScrip
 |---------|------|
 | [Wails v2](https://wails.io) | Frameless webview with Go<->JS bindings |
 | [Cobra](https://github.com/spf13/cobra) | CLI framework (flags, subcommands, help) |
-| [google/deck](https://github.com/google/deck) | Structured logging (stderr, Windows Event Log, syslog) |
+| [google/deck](https://github.com/google/deck) | Leveled logging (stderr, Windows Event Log, syslog) |
 | [gRPC](https://grpc.io) | Service<->CLI and service<->UI communication |
 | [fsnotify](https://github.com/fsnotify/fsnotify) | Cross-platform filesystem event monitoring |
 
@@ -355,16 +355,15 @@ Positioning is handled entirely from Go using Wails runtime APIs. This avoids DP
 The algorithm uses `WindowCenter()` as a reference point, then derives the notification corner:
 
 1. `WindowCenter()` — Wails handles DPI scaling, work area, and multi-monitor
-2. `WindowGetPosition()` → centered position `(cx, cy)`
-3. `WindowSetPosition(0, 0)` → probe the coordinate origin `(ox, oy)`
-4. Right-aligned: `x = 2*(cx-ox) - margin`
-5. Bottom-aligned (Windows) or top-aligned (macOS/Linux): `y = 2*(cy-oy) - margin`
+2. `WindowGetPosition()` → centered position `(cx, cy)`, `WindowGetSize()` → `(w, h)`
+3. Derive work-area dimensions: `waW = 2*cx + w`, `waH = 2*cy + h`
+4. Right-aligned: `x = waW - w - margin`
+5. Bottom-aligned (Windows): `y = waH - h - margin`, top-aligned (macOS/Linux): `y = margin`
 
 | Platform | Corner | Why |
 |----------|--------|-----|
 | Windows | Bottom-right | Matches Action Center / native toasts |
-| macOS | Top-right | Cocoa y-axis: origin at bottom-left, `y = oy + margin` places window just below menu bar |
-| Linux | Top-right | GTK y-down: `y = oy + margin` from top edge |
+| macOS/Linux | Top-right | Wails normalizes native coordinate systems, `y = margin` from top edge |
 
 ---
 

docs/usage.md

@@ -66,7 +66,7 @@ hermes accepts a single JSON or YAML config with these fields:
 | `buttons` | array | no | `[]` | Button definitions (see below) |
 | `timeout` | int | no | `300` | Seconds until auto-action |
 | `timeout_value` | string | no | `""` | Value returned on timeout |
-| `esc_value` | string | no | `""` | Value returned on ESC (defaults to `timeout_value`) |
+| `esc_value` | string | no | `""` | Value returned on ESC (defaults to `timeout_value` when set, otherwise empty) |
 | `title` | string | no | `IT Department` | Small uppercase label at the top |
 | `accent_color` | string | no | `#D4A843` | Theme accent color (hex) |
 | `help_url` | string | no | `""` | "Need help?" link URL |
@@ -363,6 +363,7 @@ The second notification is held in `waiting_on_dependency` state until the first
 | `hermes install` | Configure MOTD hook and launch daemon in active user sessions (when elevated). Called by package postinstall. |
 | `hermes uninstall` | Remove MOTD hook. Called by package removal scripts. |
 | `hermes stop` | Graceful daemon shutdown (gRPC then fallback kill) |
+| `hermes motd` | Print pending notification summary for SSH login banners (called by profile.d scripts) |
 | `hermes demo` | Show a demo notification |
 | `hermes version` | Print version, build date, Go, and OS info |
 
@@ -375,8 +376,8 @@ The second notification is held in `waiting_on_dependency` state until the first
 | `--config <path or json>` | root | config file or inline JSON/YAML — routes to service |
 | `--local` | root | Render locally in current session (skip service) |
 | `--locale <code>` | root | Override locale for localized notifications (e.g. `ja`, `de`) |
-| `--port <int>` | serve, notify, list, cancel | gRPC port (default: 4770) |
-| `--db <path>` | serve, inbox | Bolt database path (default: platform-specific, see [Architecture](architecture.md#persistence)) |
+| `--port <int>` | serve, notify, list, cancel, stop, inbox | gRPC port (default: 4770) |
+| `--db <path>` | serve, inbox, motd | Bolt database path (default: platform-specific, see [Architecture](architecture.md#persistence)) |
 | `--json` | inbox | Print history as JSON instead of opening the UI |
 | `--help` | all | Print help |
 

frontend/main.js

@@ -354,7 +354,7 @@
     if (!document.hasFocus()) return;
     if (e.keyCode === 27) respond(escValue);
     if (e.keyCode === 13) {
-      var primary = document.querySelector(".btn-primary");
+      var primary = document.querySelector(".btn-primary[data-value]");
       if (primary) respond(primary.getAttribute("data-value"));
     }
     if (carouselTotal > 1) {

internal/action/action.go

@@ -65,11 +65,6 @@ func AllowedOn(value, goos string) bool {
 	return false
 }
 
-// Classify returns the Kind of a value on the current OS.
-func Classify(value string) Kind {
-	return ClassifyOn(value, runtime.GOOS)
-}
-
 // ClassifyOn returns the Kind of a value on the given OS.
 func ClassifyOn(value, goos string) Kind {
 	lower := strings.ToLower(value)

internal/app/app.go

@@ -39,19 +39,15 @@ type App struct {
 }
 
 // New creates the App with the parsed config (local mode).
+// Caller must call cfg.ApplyDefaults() first (sets Platform if empty).
 func New(cfg *config.NotificationConfig) *App {
-	if cfg.Platform == "" {
-		cfg.Platform = goRuntime.GOOS
-	}
 	return &App{cfg: cfg, deferAllowed: true}
 }
 
 // NewWithGRPC creates the App in service mode. The gRPC client is used to
 // report the user's choice back to the service daemon.
+// Caller must call cfg.ApplyDefaults() first (sets Platform if empty).
 func NewWithGRPC(cfg *config.NotificationConfig, gc grpcReporter, notifID string, deferAllowed bool) *App {
-	if cfg.Platform == "" {
-		cfg.Platform = goRuntime.GOOS
-	}
 	return &App{
 		cfg:            cfg,
 		grpcClient:     gc,

internal/app/app_test.go

@@ -10,6 +10,7 @@ import (
 func TestNew(t *testing.T) {
 	t.Parallel()
 	cfg := &config.NotificationConfig{Heading: "Test", Message: "Body"}
+	cfg.ApplyDefaults()
 	a := New(cfg)
 	if a.cfg != cfg {
 		t.Error("cfg not set")
@@ -25,6 +26,7 @@ func TestNew(t *testing.T) {
 func TestNewWithGRPC(t *testing.T) {
 	t.Parallel()
 	cfg := &config.NotificationConfig{Heading: "Test", Message: "Body"}
+	cfg.ApplyDefaults()
 	a := NewWithGRPC(cfg, nil, "notif-1", false)
 
 	if a.notificationID != "notif-1" {

internal/client/client.go

@@ -82,7 +82,7 @@ func (c *Client) GetUIConfig(ctx context.Context, notificationID string) (*confi
 	if err != nil {
 		return nil, false, fmt.Errorf("get ui config rpc: %w", err)
 	}
-	cfg, err := config.LoadJSON(resp.ConfigJson)
+	cfg, err := config.Load(resp.ConfigJson)
 	if err != nil {
 		return nil, false, fmt.Errorf("parse config from service: %w", err)
 	}
@@ -177,14 +177,6 @@ func (c *Client) ListHistory(ctx context.Context) ([]HistoryEntry, error) {
 	return out, nil
 }
 
-// Ping attempts a quick List RPC to check if the service is running.
-func (c *Client) Ping(ctx context.Context) error {
-	ctx, cancel := context.WithTimeout(ctx, 2*time.Second)
-	defer cancel()
-	_, err := c.svc.List(ctx, &pb.ListRequest{})
-	return err
-}
-
 // Shutdown requests a graceful daemon shutdown via gRPC.
 func (c *Client) Shutdown(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, 5*time.Second)

internal/client/client_test.go

@@ -51,8 +51,8 @@ func TestDialAndPing(t *testing.T) {
 	}
 	defer c.Close()
 
-	if err := c.Ping(context.Background()); err != nil {
-		t.Errorf("Ping: %v", err)
+	if _, err := c.List(context.Background()); err != nil {
+		t.Errorf("List: %v", err)
 	}
 }