fix(goals): strip Copilot launcher preambles from goal slugs

[rysweet]

Summary

Prevent Copilot CLI launcher output from leaking into goal IDs and derived engineer/<slug> branch names.

• strip only narrow launcher signatures before goal_slug() normalization
• reuse the existing recipe-output recognizer through a narrower goal-title predicate
• preserve legitimate INFO, WARN, update-nag, JSON, and NODE_OPTIONS prose
• add regression coverage for slug stability, collision resistance, and ref safety
• document the exact matching contract and limitations

Scope

This rework intentionally removes the unrelated dashboard timestamp and websocket-log documentation that accumulated on the stale branch. The final diff is limited to goal-slug preamble sanitization and its reference documentation.

Resolves #4376.

Validation

• cargo test --lib goals::types::tests
• cargo test --lib recipe_output::extract
• cargo test --test docs_integrity
• repository pre-commit and pre-push checks
• strict maintainer review: CLEAN

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[rysweet]

Step 17b — Comprehensive Code Review ✅ APPROVE

Reviewed the full diff (15 source/doc files, +898/−15). Verified independently: cargo build --lib clean; targeted regression suites 82/82 pass (goals::types 10, memory_cognitive 7, tests_recent_episode_timestamp 3, recipe_output::extract 62).

Fix-by-fix verification

#4376 — launcher-preamble slug sanitization (src/goals/types.rs)

• strip_launcher_preamble is applied at the slug/branch derivation chokepoint, exactly the smallest surface that protects both slug and engineer/<slug> branch refs.
• ✅ Correctly reuses the single is_copilot_launcher_line recognizer (now pub(crate) in extract.rs) rather than a bare NODE_OPTIONS substring — keeps derivation in lock-step with the decision parser and avoids over-stripping.
• ✅ Zero-alloc common path via Cow::Borrowed; owned String only when a launcher line is actually dropped.
• ✅ Over-stripping guard tested: prose "Document the NODE_OPTIONS tuning guidance" survives.
• ✅ Ref-safety tested: /, .., ~, = never survive; no leading/trailing dash.

#4383 — Recent-Memories timestamp threading

• ✅ Root cause fixed end-to-end, not just the serializer: EpisodicMemory.created_at: DateTime<Utc> → to_episode() wraps Some(e.created_at) → new additive CognitiveEpisode.created_at: Option<DateTime<Utc>> (#[serde(default)]) → dashboard emits to_rfc3339() or null.
• ✅ Verified upstream type: amplihack_memory::EpisodicMemory.created_at is non-optional DateTime<Utc>, so Some(...) is correct and always populated (not structurally-null anymore).
• ✅ Back-compat: legacy payloads without the key deserialize to None (test proves no panic).
• ✅ Newest-first ordering preserved (derives from temporal_index sort, serializer is a pure order-preserving map — tested with mixed present/absent timestamps).

azlin#1056 — WSS token redaction

• Correctly scoped out of this Simard PR (target repo rysweet/azlin). Only the reference doc wss-connect-log-token-redaction.md lands here; no Simard source touched. ✅ No scope leak.

Checklist

• Code quality & standards — idiomatic, doc comments encode security rationale + invariants
• Test coverage adequate — simple + edge + back-compat + regression, all TDD (fail-before/pass-after)
• No TODOs, stubs, or swallowed exceptions — all unwrap()/panic! confined to test code
• No unimplemented functions
• Logic correctness — verified upstream types & full data path
• Edge cases — preamble-only, over-stripping, legacy deserialization, ordering, ref-safety

Non-blocking observations

1. Preamble-only input → empty slug (goal_slug("<preamble only>") == ""). This matches the pre-existing contract (a punctuation-only title already produced "") and the doc states it is caller-handled, so it is not a regression. A future defensive fallback (e.g. hash-based slug when empty) would harden pathological callers, but is out of scope here.
2. is_copilot_launcher_line widened to pub(crate) — good DRY; keeps a single source of truth for launcher recognition shared by the parser and the sanitizer.

Verdict: APPROVE — no blocking findings. Change set is complete, correct, additive/back-compatible, and green.

[rysweet]

Step 17c — Security Review ✅ PASS (zero blocking findings)

Threat-modeled the full diff (15 files, +898/−15) against injection, sensitive-data handling, and authn/authz. This change set is net credential-hygiene positive — it closes a data-leak path and documents another.

Security requirements — verified

Area	Finding
Injection / ref-safety	goal_slug normalizes to [A-Za-z0-9._-]; the new strip_launcher_preamble runs before slugification. Tests assert derived engineer/<slug> refs never contain /, .., ~, or = (path-traversal / git-ref-injection safe). No SQL/shell/format-string sinks introduced.
Sensitive-data handling	Root purpose of #4376: prevent the launcher saved-preference preamble (host config path /home/azureuser/.amplihack/config, NODE_OPTIONS tokens) from leaking into goal IDs, branch names, and git remotes. Regression test goal_slug_preamble_does_not_leak_config_path_or_env_tokens enforces the negative property for 10 distinct tokens. Removes a real info-disclosure vector.
Over-stripping (availability)	The strip reuses the single anchored recognizer is_copilot_launcher_line, which requires the full launcher signature (e.g. ℹ + NODE_OPTIONS= + (saved preference)), not a bare substring. goal_slug_prose_mentioning_node_options_is_preserved guards against a denial-of-legitimate-input regression. JSON/{/"/[-leading and ISO-timestamped payload lines are explicitly never dropped (#2570 guard intact).
ReDoS / DoS	Recognizer is plain starts_with/contains/byte-match — no regex, no backtracking, linear in input length. strip_launcher_preamble is single-pass with zero-alloc borrow on the common (no-preamble) path. No unbounded growth.
Authn / authz	Not applicable — no auth, permission, or trust-boundary code touched.
Secret exposure in new code	Grepped all added production lines: no token/secret/password/api_key/bearer handling introduced. unwrap()/panic! occurrences are confined to #[cfg(test)] modules.
Timestamp threading (#4383)	EpisodicMemory.created_at → CognitiveEpisode.created_at → RFC3339 serializer carries only non-sensitive DateTime<Utc> values; no PII/credential surface.

Cross-repo credential fix (documentation-only here)

docs/reference/wss-connect-log-token-redaction.md documents azlin#1056 — a websocket_token-in-wss_url log leak fixed in the separate rysweet/azlin checkout. No azlin code lands in this Simard PR, so nothing to verify in-tree; the doc correctly records the redaction contract (scheme+host+path retained, token value replaced with a fixed marker, no partial disclosure, no Debug leak, structured-tracing only). Scoping is correct.

Independent verification

• cargo build --lib clean.
• Targeted security-relevant regression suites green: goals::types (incl. all 5 Copilot launcher preamble (ℹ NODE_OPTIONS=… saved preference) leaks into goal slugs / branch names #4376 preamble/ref-safety tests), recipe_output::extract, dashboard timestamp tests — 82/82 pass.

Conclusion

No new vulnerabilities. Two credential/info-disclosure paths are closed or documented. Injection, ReDoS, and over-stripping edges are test-guarded. APPROVE from a security standpoint — no blocking findings.

[rysweet]

Step 17d — Philosophy Guardian Review ✅ PASS

Reviewed the full diff (15 files, +898/−15) against amplihack philosophy. Zero blocking findings.

Compliance checklist

• ✅ Ruthless simplicity — Each fix is the minimal change at the correct chokepoint. Slug sanitization reuses the single is_copilot_launcher_line recognizer rather than duplicating detection logic. Dashboard timestamp threads an existing created_at value through the pipeline; no new machinery invented.
• ✅ Bricks & studs — Changes respect module boundaries: goals::types owns slug logic, operator_commands_dashboard::memory owns serialization, cognitive_memory::library_adapter owns the EpisodicMemory → CognitiveEpisode mapping. Contracts extended additively (CognitiveEpisode.created_at) without breaking existing studs.
• ✅ Zero-BS — No stubs, no faked APIs, no swallowed exceptions. All unwrap()/panic! confined to test code; production paths handle empty/edge inputs explicitly. Verified: cargo build --lib clean, 82/82 targeted regression tests pass.
• ✅ No over-engineering — No speculative abstraction. Widening the recognizer to pub(crate) is DRY reuse, not new indirection. Zero-alloc common path avoids premature optimization while staying simple.
• ✅ Clean module boundaries — No cross-cutting leakage; the timestamp fix flows through the established adapter layer, and docs are colocated under docs/reference/ and linked from index.md.

Notes (non-blocking)

• Preamble-only input yields an empty slug — consistent with the pre-existing contract, handled by callers. Correct to leave as-is.
• The three reference docs encode the security rationale (Copilot launcher preamble (ℹ NODE_OPTIONS=… saved preference) leaks into goal slugs / branch names #4376 over-stripping invariant, token redaction) directly alongside the code intent — good regeneratable-brick discipline.

Decision: ACCEPT. The change set is simple, modular, and self-contained. Ready to finalize.

[rysweet]

Ready for Final Review

Workflow steps completed: requirements, design, implementation, tests, code review, philosophy compliance, cleanup, and quality audit.

Ready for merge approval.

[github-actions]

📊 Coverage Summary

Generated by cargo llvm-cov --workspace --summary-only (nightly, excluding test files)

Module	Lines	Covered	Coverage
Total	214644	181413	84.5%

_{Coverage data from CI run. Test files matching tests?/ are excluded from line counts.}