From f0f100a03fee41698ac5889f7c2778e16745008a Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Tue, 2 Jun 2026 08:42:24 -0700 Subject: [PATCH 01/15] harden notify resolution: drop project-local key tier, surface undeliverable webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real defects behind the "notify fails in worktrees" report (P0.8.5): - crypto.ResolveKeyPath auto-detected a project-local /.ctx.key and preferred it over the global key. That file is gitignored, so a fresh worktree resolved to a different key and decryption silently failed. Remove the tier: resolution is now key_path override > global, with project-local kept only as a degenerate fallback when no home dir exists (never auto-detected). Also a documented security antipattern (key next to ciphertext). - notify.Send swallowed every fire-path failure as a silent no-op. It now treats .notify.enc existence as the sole "configured" signal and warns (non-fatal) when a configured webhook cannot be delivered — bad/absent key, decrypt, marshal, or POST — while keeping legitimate silences (not configured, event not subscribed). LoadWebhook detects file absence via os.Stat + errors.Is, not os.IsNotExist, which does not unwrap the text-registry-wrapped error. Spec: specs/notify-resolution-hardening.md Signed-off-by: Jose Alekhinne --- .context/DECISIONS.md | 56 +++++ .context/LEARNINGS.md | 11 + .context/TASKS.md | 23 +- docs/recipes/parallel-worktrees.md | 33 ++- internal/cli/notify/doc.go | 10 + internal/config/warn/warn.go | 22 ++ internal/crypto/keypath.go | 35 +-- internal/crypto/keypath_test.go | 37 ++-- internal/log/warn/warn.go | 16 ++ internal/notify/notify.go | 84 +++++--- internal/notify/notify_test.go | 174 ++++++++++++++- internal/rc/rc.go | 10 +- site/cli/context/index.html | 18 ++ site/recipes/parallel-worktrees/index.html | 31 ++- site/search.json | 2 +- specs/notify-resolution-hardening.md | 239 +++++++++++++++++++++ 16 files changed, 714 insertions(+), 87 deletions(-) create mode 100644 specs/notify-resolution-hardening.md diff --git a/.context/DECISIONS.md b/.context/DECISIONS.md index 22c51450f..5c83de670 100644 --- a/.context/DECISIONS.md +++ b/.context/DECISIONS.md @@ -3,6 +3,7 @@ | Date | Decision | |----|--------| +| 2026-06-02 | Remove the implicit project-local .ctx.key resolution tier | | 2026-05-30 | Name the add JSON-ingest flag --json-file, not --json | | 2026-05-28 | ctxctl PATH-installed alongside ctx for clean roots and one binary across worktrees | | 2026-05-28 | Memory pressure detection uses OS-native signals (macOS pressure level + Linux PSI), not occupancy | @@ -158,6 +159,61 @@ For significant decisions: --> +## [2026-06-02-051330] Remove the implicit project-local .ctx.key resolution tier + +**Status**: Accepted + +**Context**: Picking up TASKS.md P0.8.5 ("notify fails in worktrees"), we +found `crypto.ResolveKeyPath` still auto-detects a project-local +`/.ctx.key` (a stat-gated tier) and prefers it over the global +`~/.ctx/.ctx.key`. That file is gitignored, so it is absent in a fresh +worktree checkout: resolution silently falls back to the (different) global +key and webhook/pad decryption fails. The v0.8.0 global-encryption-key spec +already collapsed per-project keys into one global key — calling project-local +keys "a security antipattern [key next to ciphertext]" that "broke in +worktrees" — but left the implicit auto-detect tier in place. Empirically +(built binary + isolated repo + fake webhook sink): the default global key +works in worktrees; only a project-local key reproduces the failure, and the +fire path swallows it silently. + +**Alternatives Considered**: +- Approach A — worktree-aware key fallback via `git rev-parse --git-common-dir` + to resolve the main checkout's key from inside a worktree: keeps project-local + keys working / but adds git-awareness to key resolution, contradicts the + CWD-anchored model, larger blast radius, and props up a deprecated mechanism. +- Approach B — copy the key into the worktree at creation (ctx-worktree skill): + no resolver change / but agent-driven and unenforceable, widens skill + permissions, and is redundant under a global key. +- Keep the tier, only fix the silent failure: smallest change / but leaves the + documented security antipattern and the worktree divergence in place. + +**Decision**: Remove the implicit project-local `.context/.ctx.key` +auto-detection tier from `ResolveKeyPath`. Resolution becomes: (1) explicit +`.ctxrc key_path` override, (2) global `~/.ctx/.ctx.key`, (3) project-local +path only as a degenerate fallback when the home dir is unavailable. Genuine +per-project isolation stays available via the explicit `key_path` override. +Paired with surfacing the silent fire-path failure so any stranded-key decrypt +failure is visible, not silent. + +**Rationale**: The project-local key is the only thing that makes a worktree +behave differently from N side-by-side terminals in the same directory; +removing it makes them indistinguishable (the desired model) and deletes a +security antipattern the project already named. It is net deletion, consistent +with the global-key and cwd-anchored simplifications. The explicit override +covers the rare real isolation need without the ciphertext-adjacent footgun. + +**Consequence**: Projects on the default global key are unaffected. Projects +with a project-local `.context/.ctx.key` resolve to the global key; their +existing local-key-encrypted `.notify.enc` / pad will fail to decrypt — now +visibly (a warning on the fire path, a surfaced error on pad/test paths) +instead of silently. Documented remedy: back up the local key, then re-key to +global or set an explicit `key_path`. No auto-migration (none exists in-tree). + +**Related**: Spec specs/notify-resolution-hardening.md | Supersedes the +project-local auto-detection portion of +specs/released/v0.8.0/global-encryption-key.md | Relates to +specs/cwd-anchored-context.md and [2026-03-01] Global encryption key + ## [2026-05-30-114429] Name the add JSON-ingest flag --json-file, not --json **Status**: Accepted diff --git a/.context/LEARNINGS.md b/.context/LEARNINGS.md index 3120d53c1..da16cea30 100644 --- a/.context/LEARNINGS.md +++ b/.context/LEARNINGS.md @@ -17,6 +17,7 @@ DO NOT UPDATE FOR: | Date | Learning | |----|--------| +| 2026-06-02 | os.IsNotExist doesn't unwrap; detect file absence with os.Stat + errors.Is(os.ErrNotExist) | | 2026-06-01 | An error-discard catalogue is an inventory, not a verdict — verify each site by reading before fixing | | 2026-06-01 | Guard managed blocks before regenerating; don't trust the span to be machine-owned | | 2026-05-30 | Capture golden fixtures from the live legacy code path before deleting it | @@ -171,6 +172,16 @@ DO NOT UPDATE FOR: --- +## [2026-06-02-051330] os.IsNotExist doesn't unwrap — detect file absence with os.Stat + errors.Is + +**Context**: Hardening notify (P0.8.5), `LoadWebhook` needed to tell "encrypted file genuinely absent" (silent: not configured) from "present but broken" (surface it). `os.IsNotExist(loadErr)` on `crypto.LoadKey`'s error is always false: `LoadKey` wraps the os error via `errCrypto.ReadKey` → `fmt.Errorf(desc.Text(...), cause)`, and `os.IsNotExist` does not unwrap. The subtle part is `errors.Is(loadErr, os.ErrNotExist)`: it is **registry-dependent**. `errCrypto.ReadKey`'s format string comes from the externalized text registry (`'read key: %w'`); `fmt.Errorf` honors `%w` at runtime regardless of where the string came from, so in production (registry loaded) `errors.Is` correctly unwraps to `fs.ErrNotExist`. But in a unit-test binary that never initializes the text registry (verified: a probe in `internal/notify`), `desc.Text` returns a string with **no** `%w`, the cause is never wrapped, the error prints `%!(EXTRA *fs.PathError=...)`, and `errors.Is` also returns false. So the same call can behave differently in prod vs. a bare test binary. + +**Lesson**: `os.IsNotExist` is the legacy, non-unwrapping check — false on any `fmt.Errorf("…%w…", …)` error; always prefer `errors.Is`. But `errors.Is(err, os.ErrNotExist)` only holds if the wrap actually carries `%w` at runtime, and a wrap whose format string is fetched from a text/i18n registry only carries `%w` when that registry is initialized. `go vet`'s wrap check sees only literal format strings, so a registry-templated wrap is vet-invisible and its wrapping is environment-dependent. + +**Application**: To detect file absence reliably, stat the path directly: `os.Stat` returns an unwrapped `*fs.PathError`, so `errors.Is(statErr, os.ErrNotExist)` is dependable in every context. Branch on the stat (absent → not-configured; present → proceed to read/decrypt and surface any error). Reserve `errors.Is(…, os.ErrNotExist)` on a *returned library* error for chains you have confirmed wrap with `%w` independent of registry state. + +--- + ## [2026-06-01-195111] An error-discard catalogue is an inventory, not a verdict — verify each site by reading before fixing **Context**: Phase EH audited ~184 silent error-discard sites under internal/. The catalogue was built by grep + pattern/name classification (e.g. 'x, _ := SomethingMarshal' => B-marshal). When fixing, several name-inferred verdicts were wrong. diff --git a/.context/TASKS.md b/.context/TASKS.md index 8b76b1dcf..56cd4adbe 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -299,13 +299,22 @@ Important things that agent (or human) yeeted to the future. + static Zensical + LoopScript + Tier-2 recall HTML (metaTable/details) migrated to embedded templates behind handles; Tier-3 single-line format strings, pure joins, and the RecallListRow meta-format kept as fmt.Sprintf. -- [ ] P0.8.5: Enable webhook notifications in worktrees. Currently `ctx notify` - silently fails because `.context.key` is gitignored and absent in - worktrees. For autonomous runs with opaque worktree agents, notifications - are the one feature that would genuinely be useful. Possible approaches: - resolve the key via `git rev-parse --git-common-dir` to find the main - checkout, or copy the key into worktrees at creation time (ctx-worktree - skill). #priority:medium #added:2026-02-22 +- [x] P0.8.5: Harden notify resolution (reframed 2026-06-02). The original + premise ("`ctx notify` silently fails in worktrees because the key is + gitignored and absent") was investigated and largely disproven: with the + default global key, notify works in worktrees (verified against a built + binary + isolated repo + fake webhook sink). The failure only reproduces + with a deprecated project-local key. Real defects to fix: (1) remove the + implicit `.context/.ctx.key` resolution tier — the sole worktree-divergence + and a documented security antipattern; (2) surface the silent fire-path + failure when a CONFIGURED webhook can't be delivered (decrypt/read/POST), + while keeping legitimate silences (not-configured, event-not-subscribed). + Whether config reaches a worktree is the user's call via `.ctxrc` + git-tracking — ctx does not special-case worktrees (it cannot distinguish a + worktree from N side-by-side terminals). Approaches A (--git-common-dir key + fallback) and B (copy key at worktree creation) rejected; see DECISIONS. + Spec: specs/notify-resolution-hardening.md + #priority:medium #added:2026-02-22 #reframed:2026-06-02 - [ ] P0.9.2: Split cli-reference.md (1633 lines) into command group pages: cli-overview, cli-init-status, cli-context, cli-recall, cli-tools, cli-system — diff --git a/docs/recipes/parallel-worktrees.md b/docs/recipes/parallel-worktrees.md index 2678aced2..dbf516514 100644 --- a/docs/recipes/parallel-worktrees.md +++ b/docs/recipes/parallel-worktrees.md @@ -159,10 +159,35 @@ prompts work: ## What Works Differently in Worktrees -The encryption key lives at `~/.ctx/.ctx.key` (user-level, outside -the project). Because all worktrees on the same machine share this path, -**`ctx pad` and `ctx hook notify` work in worktrees automatically** - no -special setup needed. +The encryption key lives at `~/.ctx/.ctx.key` (user-level, outside the +project). All worktrees on the same machine share this one key — there +is no per-project key (an implicit `.context/.ctx.key` is never +auto-detected), so **`ctx pad` and `ctx hook notify` decrypt correctly +in worktrees automatically**, with no special setup. + +**Whether `ctx hook notify` actually *fires* in a worktree is your +call, made through one decision: do you git-track `.ctxrc`?** + +* **Tracked `.ctxrc`** (committed) → its `notify.events` list rides + into every checkout, so notifications fire from worktrees too. + Committing `.ctxrc` is safe: it holds `notify.events`, `key_path`, + and rotation settings — never the webhook secret, which stays + encrypted in `.context/.notify.enc`. +* **Gitignored `.ctxrc`** (e.g. the profile workflow with tracked + `.ctxrc.base` / `.ctxrc.dev`) → a fresh worktree has no active + `.ctxrc`, so ctx applies built-in defaults and notifications stay + off there. `.ctxrc.base` is a *template*, not a fallback: ctx reads + only the active `.ctxrc`. To enable notifications in such a + worktree, copy a `.ctxrc` into it (or run `ctx config switch`). + +ctx deliberately does not special-case worktrees — it cannot tell a +worktree from several terminals open in the same project — so the +`.ctxrc`-tracking choice is the single, explicit control. + +If a configured webhook ever can't be delivered (a wrong or missing +key, a decrypt failure, a network error), `ctx hook notify` prints a +`ctx: notify: webhook configured but undeliverable: …` warning to +stderr instead of silently dropping the notification. One thing to watch: diff --git a/internal/cli/notify/doc.go b/internal/cli/notify/doc.go index 9be78c878..b2bbf8e20 100644 --- a/internal/cli/notify/doc.go +++ b/internal/cli/notify/doc.go @@ -29,6 +29,16 @@ // verify connectivity without subscribing the test // event first. See [internal/cli/notify/cmd/test]. // +// # Worktrees and parallel checkouts +// +// `ctx hook notify` does not special-case git worktrees — it +// cannot distinguish one from several terminals open in the same +// project. Whether the `notify.events` filter reaches a worktree +// depends on whether `.ctxrc` is git-tracked (committed → fires +// everywhere; gitignored → worktrees fall back to defaults). The +// key is the single global `~/.ctx/.ctx.key`, shared by all +// checkouts. See `docs/recipes/parallel-worktrees.md`. +// // # Concurrency // // Stateless. The CLI command spawns one HTTP request diff --git a/internal/config/warn/warn.go b/internal/config/warn/warn.go index 3015d7ea9..daa9d9999 100644 --- a/internal/config/warn/warn.go +++ b/internal/config/warn/warn.go @@ -165,6 +165,28 @@ const ( PadHistoryPrune = "pad history: prune: %v" ) +// Notify webhook delivery warning formats. These fire only when a +// webhook IS configured but cannot be delivered — never when notify +// is simply unconfigured or the event is unsubscribed. Surfacing +// them keeps `ctx hook notify` honest: a webhook the user set up +// that silently drops (e.g. a project-local key absent in a git +// worktree, so decryption fails) reads as "working" when it is not. +const ( + // NotifyWebhookLoad is the format for a configured webhook that + // could not be loaded or decrypted: an unreadable/wrong key, a + // decrypt failure, or a resolver error. Takes (error). + NotifyWebhookLoad = "notify: webhook configured but undeliverable: %v" + + // NotifyWebhookMarshal is the format for a payload marshal + // failure on the notify fire path. Takes (error). + NotifyWebhookMarshal = "notify: marshal payload: %v" + + // NotifyWebhookPost is the format for an HTTP POST failure when + // delivering a notification (fire-and-forget, but visible). + // Takes (error). + NotifyWebhookPost = "notify: webhook POST failed: %v" +) + // Warn context identifiers for index generation. const ( // IndexHeader is the context label for index header write errors. diff --git a/internal/crypto/keypath.go b/internal/crypto/keypath.go index 894f9195c..0226d162f 100644 --- a/internal/crypto/keypath.go +++ b/internal/crypto/keypath.go @@ -56,11 +56,18 @@ func ExpandHome(path string) string { // ResolveKeyPath determines the effective key file path. // // Resolution order: -// 1. overridePath if non-empty (explicit .ctxrc key_path, -// with tilde expansion) -// 2. Project-local path if it exists (/.ctx.key) -// 3. Global default (~/.ctx/.ctx.key) -// 4. Project-local path as fallback (when home dir unavailable) +// 1. overridePath if non-empty (explicit .ctxrc key_path, with +// tilde expansion) — the supported per-project isolation knob +// 2. Global default (~/.ctx/.ctx.key) +// 3. Project-local path (/.ctx.key) as a degenerate +// fallback ONLY when the home directory is unavailable +// +// A project-local /.ctx.key is never auto-detected or +// preferred over the global key. That implicit tier was removed: it +// stored the key next to the ciphertext (a security antipattern) and +// was the sole cause of key divergence in git worktrees, where the +// gitignored key is absent from the checkout. Per +// specs/notify-resolution-hardening.md. // // Parameters: // - contextDir: The .context/ directory path @@ -74,18 +81,14 @@ func ResolveKeyPath(contextDir, overridePath string) string { return ExpandHome(overridePath) } - // Tier 2: project-local key. - local := filepath.Join(contextDir, cfgCrypto.ContextKey) - if _, statErr := os.Stat(local); statErr == nil { - return local - } - - // Tier 3: global default. - global := GlobalKeyPath() - if global != "" { + // Tier 2: global default. + if global := GlobalKeyPath(); global != "" { return global } - // Fallback: project-local (home dir unavailable). - return local + // Degenerate fallback: the project-local path, used only when the + // home directory is unavailable so no global location can be + // computed. This is NOT project-local key auto-detection — a stray + // /.ctx.key is never preferred over the global key. + return filepath.Join(contextDir, cfgCrypto.ContextKey) } diff --git a/internal/crypto/keypath_test.go b/internal/crypto/keypath_test.go index b5e30381b..84a47acf5 100644 --- a/internal/crypto/keypath_test.go +++ b/internal/crypto/keypath_test.go @@ -62,34 +62,43 @@ func TestResolveKeyPath_OverrideTakesPrecedence(t *testing.T) { } } -func TestResolveKeyPath_ProjectLocalBeforeGlobal(t *testing.T) { +func TestResolveKeyPath_ProjectLocalIgnored(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - // Create both project-local and global keys. + // A project-local key exists, but it must NOT be auto-detected: + // the global key wins. The implicit project-local tier was removed + // (it broke worktrees and was a security antipattern) — see + // specs/notify-resolution-hardening.md. contextDir := filepath.Join(dir, "project", ".context") - if err := os.MkdirAll(contextDir, 0750); err != nil { + if err := os.MkdirAll(contextDir, fs.PermKeyDir); err != nil { t.Fatal(err) } localKey := filepath.Join(contextDir, cfgCrypto.ContextKey) - localData := []byte("local-key") - if err := os.WriteFile(localKey, localData, fs.PermSecret); err != nil { + if err := os.WriteFile(localKey, []byte("local-key"), fs.PermSecret); err != nil { t.Fatal(err) } - globalDir := filepath.Join(dir, ".ctx") - if err := os.MkdirAll(globalDir, fs.PermKeyDir); err != nil { - t.Fatal(err) + got := ResolveKeyPath(contextDir, "") + want := GlobalKeyPath() + if got != want { + t.Errorf("ResolveKeyPath() = %q, want global %q (project-local must be ignored)", got, want) } - globalKey := filepath.Join(globalDir, cfgCrypto.ContextKey) - globalData := []byte("global-key") - if err := os.WriteFile(globalKey, globalData, fs.PermSecret); err != nil { - t.Fatal(err) + if got == localKey { + t.Errorf("ResolveKeyPath() returned the project-local key %q; it must not be auto-detected", localKey) } +} +func TestResolveKeyPath_HomeUnavailableFallsBackToLocal(t *testing.T) { + // With no home dir, GlobalKeyPath() returns "" and resolution + // falls back to the project-local path as a last resort. + t.Setenv("HOME", "") + + contextDir := filepath.Join("project", ".context") got := ResolveKeyPath(contextDir, "") - if got != localKey { - t.Errorf("ResolveKeyPath() = %q, want project-local %q", got, localKey) + want := filepath.Join(contextDir, cfgCrypto.ContextKey) + if got != want { + t.Errorf("ResolveKeyPath() = %q, want local fallback %q", got, want) } } diff --git a/internal/log/warn/warn.go b/internal/log/warn/warn.go index d88abfa84..896fbfa93 100644 --- a/internal/log/warn/warn.go +++ b/internal/log/warn/warn.go @@ -35,3 +35,19 @@ func Warn(format string, args ...any) { _, _ = fmt.Fprintf( sink, cfgCtx.StderrPrefix+format+token.NewlineLF, args...) } + +// SetSink redirects warning output to w and returns a function that +// restores the sink in effect before the call. It exists so tests can +// capture or discard warnings; production code never calls it. Callers +// must not invoke it from parallel tests — sink is process-global. +// +// Parameters: +// - w: writer to receive subsequent warnings +// +// Returns: +// - func(): restores the previous sink +func SetSink(w io.Writer) func() { + prev := sink + sink = w + return func() { sink = prev } +} diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 2a10a50f3..551882644 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -8,6 +8,7 @@ package notify import ( "encoding/json" + "errors" "net/http" "os" "path/filepath" @@ -27,22 +28,20 @@ import ( // LoadWebhook reads and decrypts the webhook URL from .context/.notify.enc. // -// Returns ("", nil) when: -// - the key file is missing (key was never generated), -// - the encrypted file is missing (webhook never configured). -// -// Any resolver or I/O failure is propagated (including -// [errCtx.ErrNoCtxHere]) so callers can distinguish -// "no context dir" from "no webhook configured" rather than -// being forced to treat them identically. [Send] treats any error -// as "no webhook, silently skip"; interactive callers (e.g. -// `ctx notify test`) can use [errors.Is] to surface a clearer -// message when the project is not set up yet. +// The webhook is "configured" exactly when .context/.notify.enc exists. +// Its absence is the one silent "not configured" signal: LoadWebhook +// returns ("", nil). Once the encrypted file exists, anything that then +// prevents decryption is a real problem and is propagated: a missing, +// unreadable, or invalid key (e.g. a project-local key absent in a git +// worktree), an unreadable .notify.enc, a decryption failure (wrong +// key), or a resolver error such as [errCtx.ErrNoCtxHere]. This lets +// callers distinguish "not configured" (silent) from "configured but +// broken" (surface it). [Send] warns on the latter; interactive callers +// (e.g. `ctx hook notify test`) report it directly. // // Returns: // - string: the decrypted webhook URL, or "" if not configured -// - error: non-nil on any resolver failure or decryption failure; -// missing key / encrypted file are silent +// - error: non-nil when a configured webhook cannot be decrypted func LoadWebhook() (string, error) { kp, kpErr := rc.KeyPath() if kpErr != nil { @@ -54,22 +53,29 @@ func LoadWebhook() (string, error) { } encPath := filepath.Join(ctxDir, cfgCrypto.NotifyEnc) - key, loadErr := crypto.LoadKey(kp) - if loadErr != nil { - if os.IsNotExist(loadErr) { - return "", nil + // A missing .notify.enc is the only silent "not configured" case. + // os.Stat returns an unwrapped *fs.PathError, so this not-exist + // check is reliable regardless of how downstream library errors are + // wrapped (crypto.LoadKey wraps through the text registry, on which + // neither os.IsNotExist nor errors.Is is dependable). Once the + // encrypted file exists the webhook IS configured, so a missing, + // unreadable, or invalid key — and any decrypt failure — is surfaced + // rather than mistaken for "no webhook". + if _, statErr := os.Stat(encPath); statErr != nil { + if errors.Is(statErr, os.ErrNotExist) { + return "", nil // webhook never configured } - return "", nil + return "", statErr } + key, loadErr := crypto.LoadKey(kp) + if loadErr != nil { + return "", loadErr // configured, but key missing/unreadable/invalid + } ciphertext, readErr := io.SafeReadUserFile(encPath) if readErr != nil { - if os.IsNotExist(readErr) { - return "", nil - } - return "", nil + return "", readErr // enc present but unreadable } - plaintext, decryptErr := crypto.Decrypt(key, ciphertext) if decryptErr != nil { return "", decryptErr @@ -146,10 +152,18 @@ func EventAllowed(event string, allowed []string) bool { return false } -// Send fires a webhook notification. It is a silent noop when: -// - no webhook URL is configured -// - the event is not in the allowed list -// - the HTTP request fails (fire-and-forget) +// Send fires a webhook notification. It is a silent noop only when +// delivery is not expected: +// - the event is not in the allowed list (not subscribed), or +// - no webhook URL is configured. +// +// When a webhook IS configured but cannot be delivered — an +// unreadable or wrong key, a decrypt failure (e.g. a project-local +// key absent in a git worktree), a marshal error, or an HTTP failure +// — Send emits a non-fatal warning to stderr and returns nil. It +// never returns a delivery error (fire-and-forget), but it is never +// silent about a real failure: a webhook the user set up that drops +// without a trace reads as "working" when it is not. // // Parameters: // - event: notification category (e.g. "relay", "nudge") @@ -158,16 +172,23 @@ func EventAllowed(event string, allowed []string) bool { // - detail: structured template reference (nil omits the field) // // Returns: -// - error: Delivery error, or nil if sent successfully or silently skipped +// - error: always nil; failures are warned, not returned func Send(event, message, sessionID string, detail *entity.TemplateRef) error { if !EventAllowed(event, rc.NotifyEvents()) { return nil } url, webhookErr := LoadWebhook() - if webhookErr != nil || url == "" { + if webhookErr != nil { + // Configured but undeliverable (wrong/absent key in a + // worktree, unreadable key, or decrypt failure). Surface it, + // but stay non-fatal (fire-and-forget). + logWarn.Warn(cfgWarn.NotifyWebhookLoad, webhookErr) return nil } + if url == "" { + return nil // not configured: legitimate silent no-op + } projectName := project.FallbackName if cwd, cwdErr := os.Getwd(); cwdErr == nil { @@ -182,12 +203,15 @@ func Send(event, message, sessionID string, detail *entity.TemplateRef) error { body, marshalErr := json.Marshal(payload) if marshalErr != nil { + logWarn.Warn(cfgWarn.NotifyWebhookMarshal, marshalErr) return nil } resp, postErr := PostJSON(url, body) if postErr != nil { - return nil // fire-and-forget + // Delivery failed: fire-and-forget, but no longer silent. + logWarn.Warn(cfgWarn.NotifyWebhookPost, postErr) + return nil } if closeErr := resp.Body.Close(); closeErr != nil { logWarn.Warn(cfgWarn.CloseResponse, closeErr) diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index 1738129c3..8d4bca8db 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -7,15 +7,18 @@ package notify import ( + "bytes" "encoding/json" "net/http" "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/ActiveMemory/ctx/internal/config/crypto" "github.com/ActiveMemory/ctx/internal/entity" + logWarn "github.com/ActiveMemory/ctx/internal/log/warn" "github.com/ActiveMemory/ctx/internal/rc" "github.com/ActiveMemory/ctx/internal/testutil/testctx" ) @@ -52,9 +55,16 @@ func TestLoadWebhook_NoFile(t *testing.T) { tempDir, cleanup := setupTestDir(t) defer cleanup() - // Create key but no encrypted file - keyPath := filepath.Join(tempDir, ".context", crypto.ContextKey) - _ = os.WriteFile(keyPath, make([]byte, 32), 0o600) + // Create the (global) key but no encrypted file. The key resolves + // to ~/.ctx/.ctx.key — HOME is redirected to tempDir in tests. + globalDir := filepath.Join(tempDir, ".ctx") + if err := os.MkdirAll(globalDir, 0o700); err != nil { + t.Fatal(err) + } + keyPath := filepath.Join(globalDir, crypto.ContextKey) + if err := os.WriteFile(keyPath, make([]byte, 32), 0o600); err != nil { + t.Fatal(err) + } url, err := LoadWebhook() if err != nil { @@ -65,6 +75,51 @@ func TestLoadWebhook_NoFile(t *testing.T) { } } +func TestLoadWebhook_InvalidKeyPropagated(t *testing.T) { + tempDir, cleanup := setupTestDir(t) + defer cleanup() + + // Key present (wrong size) AND the encrypted file present: the + // webhook IS configured, so an invalid key is a real + // misconfiguration LoadWebhook must surface, not a silent "no + // webhook". (Before the swallow was narrowed it returned ("", nil) + // and the failure vanished.) + globalDir := filepath.Join(tempDir, ".ctx") + if err := os.MkdirAll(globalDir, 0o700); err != nil { + t.Fatal(err) + } + keyPath := filepath.Join(globalDir, crypto.ContextKey) + if err := os.WriteFile(keyPath, []byte("too-short"), 0o600); err != nil { + t.Fatal(err) + } + encPath := filepath.Join(tempDir, ".context", crypto.NotifyEnc) + if err := os.WriteFile(encPath, []byte("ciphertext"), 0o600); err != nil { + t.Fatal(err) + } + + if _, err := LoadWebhook(); err == nil { + t.Fatal("LoadWebhook() with an invalid-size key: expected error, got nil") + } +} + +func TestLoadWebhook_ConfiguredKeyAbsentSurfaces(t *testing.T) { + tempDir, cleanup := setupTestDir(t) + defer cleanup() + + // .notify.enc present (webhook IS configured) but no key anywhere. + // This is "configured but broken", not "not configured", so + // LoadWebhook must surface an error rather than silently report no + // webhook (the absent-key-in-worktree / fresh-machine case). + encPath := filepath.Join(tempDir, ".context", crypto.NotifyEnc) + if err := os.WriteFile(encPath, []byte("ciphertext"), 0o600); err != nil { + t.Fatal(err) + } + + if _, err := LoadWebhook(); err == nil { + t.Fatal("LoadWebhook() with enc present but key absent: expected error, got nil") + } +} + func TestLoadWebhook_RoundTrip(t *testing.T) { _, cleanup := setupTestDir(t) defer cleanup() @@ -109,14 +164,30 @@ func TestEventAllowed_NoMatch(t *testing.T) { } func TestSend_NoWebhook(t *testing.T) { - _, cleanup := setupTestDir(t) + tempDir, cleanup := setupTestDir(t) defer cleanup() - // No webhook configured: should noop without error - err := Send("test", "hello", "session-1", nil) - if err != nil { + // Subscribe the event so Send passes the event filter and actually + // reaches the webhook-absence check, rather than short-circuiting + // at the filter. No .notify.enc exists, so this is the legitimate + // "not configured" path: noop with no error and no warning. + rcContent := "notify:\n events:\n - test\n" + if err := os.WriteFile( + filepath.Join(tempDir, ".ctxrc"), []byte(rcContent), 0o600, + ); err != nil { + t.Fatal(err) + } + rc.Reset() + + var buf bytes.Buffer + defer logWarn.SetSink(&buf)() + + if err := Send("test", "hello", "session-1", nil); err != nil { t.Fatalf("Send() error = %v", err) } + if buf.Len() != 0 { + t.Errorf("unconfigured webhook should not warn, got %q", buf.String()) + } } func TestSend_EventFiltered(t *testing.T) { @@ -280,13 +351,100 @@ func TestSend_HTTPErrorIgnored(t *testing.T) { _ = os.WriteFile(filepath.Join(tempDir, ".ctxrc"), []byte(rcContent), 0o600) rc.Reset() - // Should not return error even on HTTP 500 + // An HTTP 500 is a received response, not a transport error, so + // this exercises the success/Body.Close path. The transport-error + // (postErr) warn branch is covered by TestSend_PostFailureWarns. err := Send("test", "hello", "session-1", nil) if err != nil { t.Fatalf("Send() error = %v, want nil (fire-and-forget)", err) } } +func TestSend_ConfiguredButUndeliverableWarns(t *testing.T) { + tempDir, cleanup := setupTestDir(t) + defer cleanup() + + called := false + ts := httptest.NewServer( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + called = true + }), + ) + defer ts.Close() + + // Configure a webhook, then make it undeliverable by replacing the + // key with a different valid key so .notify.enc no longer decrypts + // — the worktree / wrong-key footgun. Send must warn (not silently + // drop) and must not POST. + if err := SaveWebhook(ts.URL); err != nil { + t.Fatalf("SaveWebhook() error = %v", err) + } + keyPath := filepath.Join(tempDir, ".ctx", crypto.ContextKey) + if err := os.WriteFile( + keyPath, bytes.Repeat([]byte{7}, 32), 0o600, + ); err != nil { + t.Fatal(err) + } + rcContent := "notify:\n events:\n - stop\n" + if err := os.WriteFile( + filepath.Join(tempDir, ".ctxrc"), []byte(rcContent), 0o600, + ); err != nil { + t.Fatal(err) + } + rc.Reset() + + var buf bytes.Buffer + defer logWarn.SetSink(&buf)() + + if err := Send("stop", "hello", "s1", nil); err != nil { + t.Fatalf("Send() error = %v, want nil (fire-and-forget)", err) + } + if called { + t.Error("webhook was POSTed despite a decrypt failure") + } + if n := strings.Count( + buf.String(), "webhook configured but undeliverable", + ); n != 1 { + t.Errorf("undeliverable warning count = %d, want 1; sink=%q", + n, buf.String()) + } +} + +func TestSend_PostFailureWarns(t *testing.T) { + tempDir, cleanup := setupTestDir(t) + defer cleanup() + + // Stand up a server, capture its URL, then close it so the POST + // gets connection-refused (a transport error, unlike an HTTP 500). + ts := httptest.NewServer( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}), + ) + url := ts.URL + ts.Close() + + if err := SaveWebhook(url); err != nil { + t.Fatalf("SaveWebhook() error = %v", err) + } + rcContent := "notify:\n events:\n - stop\n" + if err := os.WriteFile( + filepath.Join(tempDir, ".ctxrc"), []byte(rcContent), 0o600, + ); err != nil { + t.Fatal(err) + } + rc.Reset() + + var buf bytes.Buffer + defer logWarn.SetSink(&buf)() + + if err := Send("stop", "hello", "s1", nil); err != nil { + t.Fatalf("Send() error = %v, want nil (fire-and-forget)", err) + } + if n := strings.Count(buf.String(), "webhook POST failed"); n != 1 { + t.Errorf("POST-failure warning count = %d, want 1; sink=%q", + n, buf.String()) + } +} + func TestSaveWebhook_Roundtrip(t *testing.T) { _, cleanup := setupTestDir(t) defer cleanup() diff --git a/internal/rc/rc.go b/internal/rc/rc.go index 98f03ea9b..1f66fb0c9 100644 --- a/internal/rc/rc.go +++ b/internal/rc/rc.go @@ -271,9 +271,13 @@ func NotifyEvents() []string { // the absence of a project rather than rotating encryption // against a surprise key. // -// Within ResolveKeyPath the existing priority still applies: -// key_path in .ctxrc (explicit) > project-local -// (.context/.ctx.key) > global (~/.ctx/.ctx.key). +// Within ResolveKeyPath the priority is: key_path in .ctxrc +// (explicit, tilde-expanded) > global (~/.ctx/.ctx.key). The +// project-local path (.context/.ctx.key) is only a degenerate +// fallback when the home directory is unavailable, and is never +// auto-detected or preferred over the global key — see +// internal/crypto/keypath.go and +// specs/notify-resolution-hardening.md. // // Returns: // - string: Resolved path to the encryption key file diff --git a/site/cli/context/index.html b/site/cli/context/index.html index 66f352b02..87275e5c1 100644 --- a/site/cli/context/index.html +++ b/site/cli/context/index.html @@ -2213,6 +2213,11 @@

Adding entriesAdding entries # Add to specific section ctx convention add "Use kebab-case for filenames" --section "Naming" + +# Ingest a JSON payload (keeps flag-value content off the command line, +# so a value containing a permissions-denied substring still persists) +cat > /tmp/decision.json <<'EOF' +{ + "title": "Install ctx into the system PATH", + "context": "agents invoke ctx by bare name", + "rationale": "the binary belongs at /usr/local/bin so it is on PATH", + "consequence": "ctx resolves from any working directory", + "provenance": {"session_id": "abc12345", "branch": "main", "commit": "68fbc00a"} +} +EOF +ctx decision add --json-file /tmp/decision.json

ctx drift

diff --git a/site/recipes/parallel-worktrees/index.html b/site/recipes/parallel-worktrees/index.html index 1f1c00d3e..fdcf7087d 100644 --- a/site/recipes/parallel-worktrees/index.html +++ b/site/recipes/parallel-worktrees/index.html @@ -2273,10 +2273,33 @@

Conversational Approach"Clean up all the worktrees, we're done."

What Works Differently in Worktrees

-

The encryption key lives at ~/.ctx/.ctx.key (user-level, outside -the project). Because all worktrees on the same machine share this path, -ctx pad and ctx hook notify work in worktrees automatically - no -special setup needed.

+

The encryption key lives at ~/.ctx/.ctx.key (user-level, outside the +project). All worktrees on the same machine share this one key — there +is no per-project key (an implicit .context/.ctx.key is never +auto-detected), so ctx pad and ctx hook notify decrypt correctly +in worktrees automatically, with no special setup.

+

Whether ctx hook notify actually fires in a worktree is your +call, made through one decision: do you git-track .ctxrc?

+
    +
  • Tracked .ctxrc (committed) → its notify.events list rides + into every checkout, so notifications fire from worktrees too. + Committing .ctxrc is safe: it holds notify.events, key_path, + and rotation settings — never the webhook secret, which stays + encrypted in .context/.notify.enc.
    • +
  • Gitignored .ctxrc (e.g. the profile workflow with tracked + .ctxrc.base / .ctxrc.dev) → a fresh worktree has no active + .ctxrc, so ctx applies built-in defaults and notifications stay + off there. .ctxrc.base is a template, not a fallback: ctx reads + only the active .ctxrc. To enable notifications in such a + worktree, copy a .ctxrc into it (or run ctx config switch).
    • +
+

ctx deliberately does not special-case worktrees — it cannot tell a +worktree from several terminals open in the same project — so the +.ctxrc-tracking choice is the single, explicit control.

+

If a configured webhook ever can't be delivered (a wrong or missing +key, a decrypt failure, a network error), ctx hook notify prints a +ctx: notify: webhook configured but undeliverable: … warning to +stderr instead of silently dropping the notification.

One thing to watch: