diff --git a/.github/LLM_PROTOCOL_DECISIONS.md b/.github/LLM_PROTOCOL_DECISIONS.md index 2d54a24..9d98905 100644 --- a/.github/LLM_PROTOCOL_DECISIONS.md +++ b/.github/LLM_PROTOCOL_DECISIONS.md @@ -146,6 +146,8 @@ The "primary" vs "inherit" distinction is defined in `protocol-audit/references/ | LLMP-DEC-38 | 2026-04-24 | Retroactively assigned `LLMP-DEC-N` IDs to existing Decision Log entries | When the globally-unique ID system was introduced (LLMP-DEC-30) with the `LLMP` topic code for LLM-protocol meta-decisions using `DEC` type, the existing ~37 Decision Log entries remained keyed by date alone. This meant the design intention (entries have unambiguous string IDs for cross-referencing) wasn't realized for historical entries. Fix: added an `ID` column as the leftmost in the 2026 table, and populated LLMP-DEC-1 through LLMP-DEC-37 in chronological order (retroactive assignment, this entry is LLMP-DEC-38 as the first ID-aware addition). Entries now referenceable by ID from code comments, other `.md` files, PR descriptions, or future DB migration. The `LLMP-DEC-N` format is append-only (IDs never renumber); superseded decisions get new entries that reference old ones by ID (e.g., "LLMP-DEC-26 superseded by LLMP-DEC-27"). | `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` (ID column + 37 retroactive IDs) | | LLMP-DEC-39 | 2026-04-24 | `docs-discovery` SKILL.md Step 2 restructuring — recursive `**/` wildcard requirement moved to prominent CRITICAL section at TOP (second iteration; LLMP-DEC-37 proved insufficient) | **Incident**: a second Copilot session exhibited the same literal-path bug that LLMP-DEC-37 was supposed to prevent — it synthesized `AyCode.Core/docs/LOGGING/README.md` (a path that doesn't exist; the real docs are at `AyCode.Core/AyCode.Core/docs/LOGGING/`, one level deeper at project-level) instead of using the recursive `**/docs/LOGGING/**/*.md` glob. LLMP-DEC-37 had appended a warning to the END of Step 2, which was not prominent enough — Copilot read Step 2, formed its own mental model of "usual docs paths", and never reached the trailing caution. **Fix**: moved the warning to the TOP of Step 2 as a `### ⚠️ CRITICAL` subsection. Content expanded: (a) explicit depth-level enumeration (repo-root `/docs/TOPIC/`, project-level `//docs/TOPIC/`, nested), (b) concrete failure-mode walkthrough showing the specific "know the repo → synthesize shallow path → 0 matches → false-empty conclusion" pattern that fires for Pattern B layouts, (c) correct-vs-wrong form side-by-side examples, (d) absolute rule: NEVER drop the leading `**/` even when the repo is known. The old trailing warning was consolidated to a back-reference to the new top section to keep Step 2 from splitting the message. User rejected intermediate proposals (byte-count verification, flat `**/docs/TOPIC.md` defensive pattern) as "drótozás" (hardcoding) — fix stays structural, no glob pattern additions. | `docs-discovery/SKILL.md` (Step 2 restructuring; supersedes LLMP-DEC-37's Step-2-end placement) | | LLMP-DEC-40 | 2026-04-24 | Rule #1 clarification: removed false "globally unique by basename" claim for `copilot-instructions.md` | **Observation**: a parallel Claude Code self-audit session noticed a literal contradiction in Rule #1. The short-name rule line 18 said `copilot-instructions.md` is "globally unique by basename" — true for a typical session (1 active repo = 1 loaded copilot-instructions.md) but FALSE for `protocol-audit` sessions, which load all 8 `copilot-instructions.md` files simultaneously. The claim conflicts with the cross-project-disambiguation rule one line above (line 17) whenever the audit use-case fires. The LLM correctly identified the ambiguity and proposed a patch. **Fix**: replaced the parenthetical "globally unique by basename" with an explicit acknowledgment of the `protocol-audit` collision case + pointer to the already-existing cross-project-disambiguation rule above. New wording uses concrete disambiguation examples (`AyCode.Core/copilot-instructions.md`, `FruitBankHybridApp/copilot-instructions.md`). The `.github/` prefix tiltás is retained — now justified as "implicit location" rather than "already unique". **Deferred (explicit YAGNI)**: proposed new `protocol-audit` C5 invariant (semantic validation: Rule #1 basename-uniqueness claims must match the actual file-set in REPOS.md). Rejected for now — one known instance isn't enough to justify invariant surface / false-positive risk; revisit if a second similar contradiction appears. | `5× primary` `copilot-instructions.md` (Rule #1 line 18 patched) | +| LLMP-DEC-41 | 2026-04-24 | Created `adr-author` skill — 4th workspace skill, extends Session Setup pre-load from 3 to 4 | **Gap identified**: the existing skill-stack (docs-discovery, docs-check, protocol-audit) covers documentation retrieval, code↔docs drift detection, and cross-repo file-structural audit — but has no artifact-producing workflow for **pre-code architectural decisions** (greenfield repo planning, feature design, tech-stack choices, library selection, migration planning). The `{TOPIC}_TODO.md` format is an implementation queue (known scope, code ref expected, P1-P3 priority); it cannot naturally hold decision rationale, weighed alternatives, or trade-off tables. Without a dedicated artifact, design rationale tends to disappear into chat history or get flattened into TODO entries that lose the "why". **Solution**: `adr-author` skill encoding the canonical Michael Nygard ADR workflow as a structured interview (Step 1 routing → Step 2 context → Step 3 alternatives → Step 4 trade-offs → Step 5 decision → Step 6 consequences → Step 7 draft + write → Step 8 cross-refs). Tool-neutral like the other three skills. **Routing logic**: two decision-log paradigms supported — per-repo `docs/adr/NNNN-.md` Nygard-style files (product/code/domain decisions) VS workspace-level `LLMP_PROTOCOL_DECISIONS.md` table rows with ID `LLMP-DEC-N` (protocol-meta decisions). **Invocation model (new): explicit user request OR LLM-suggest-back** — LLM flags when conversation exhibits 2+ ADR-heuristics (multiple alternatives weighed, irreversible, cross-cutting, 6-18mo re-openable); user must confirm before invocation. Never auto-invoke. **Session Setup pre-load 3 → 4 skill**: consistent with existing `protocol-audit` (on-demand) pattern — the skill's content must be in context for trigger recognition. **Handoff**: `docs-check` can flag ADR-worthy observations at end-of-response; `docs-discovery` owns "how does X work" questions, `adr-author` owns "let's design X" questions. **User rationale for broader scope than greenfield-only**: user correctly pointed out ADR-worthy decisions arise in mature code too (refactors, library additions, migration paths), not just new-repo planning — the original Nygard definition supports this scope. | new: `AyCode.Core/.github/skills/adr-author/SKILL.md` + `AyCode.Core/.github/skills/adr-author/references/ADR_TEMPLATE.md`; `5× primary` `copilot-instructions.md` (Session Setup: three→four; prefix count 4→5); `3× inherit` `copilot-instructions.md` (Session Setup: three→four; prefix count 5→6); `4× non-Core primary` + `3× inherit` `copilot-instructions.md` (Shared Agent Skills intro "All three"→"All four"; new adr-author bullet added after docs-check bullet) | +| LLMP-DEC-42 | 2026-04-25 | `adr-author` SKILL.md: added "Multi-project repo case" placement guidance to Step 1 (cross-cutting vs project-scoped) | **Incident**: a Claude Code session executing `adr-author` for a bearer token design produced an excellent ADR draft but left the `docs/adr/` location ambiguous — the draft path said `docs/adr/0001-...` without resolving whether that meant `/docs/adr/` (repo root, cross-cutting placement) or `//docs/adr/` (project-scoped). The decision in question affected 2+ projects (`AyCode.Services` client + `AyCode.Services.Server`), so the placement question was real, not academic. The original SKILL.md Step 1 used `/docs/adr/...` placeholder which silently assumed single-project repos; multi-project layouts (which exist in this and likely other repos) had no explicit guidance. **Fix**: added a "Multi-project repo case" sub-section to Step 1 with three rules: (1) existing convention wins (Glob first; never fragment by creating a parallel folder); (2) no existing folder → match scope to placement (cross-cutting → highest common ancestor, typically repo root; project-scoped → that project's `docs/`); (3) ambiguous scope → ask the user explicitly before proceeding. Step 7's path reference updated to `` resolved in Step 1, removing the misleading `/docs/adr/` literal. **Generic, not hardcoded**: no specific repo names or project paths in the rules — they apply to any multi-project repo. | `adr-author/SKILL.md` (Step 1 + Step 7 updates) | ## Known follow-ups diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4374521..7ad09ea 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -60,11 +60,12 @@ You are operating in a multi-repo, documentation-first architecture. You MUST ST ## Session Setup -**Mandatory reads at session start** — in addition to this `copilot-instructions.md`, the agent MUST load the three workspace skills' `SKILL.md` files: +**Mandatory reads at session start** — in addition to this `copilot-instructions.md`, the agent MUST load the four workspace skills' `SKILL.md` files: - `docs-discovery/SKILL.md` — **reactive** (triggers on any domain question — must be ready BEFORE the first domain query arrives) - `docs-check/SKILL.md` — **reactive** (triggers at the end of every code-modifying response) - `protocol-audit/SKILL.md` — **on-demand** (triggers on explicit "audit protocol" command) +- `adr-author/SKILL.md` — **on-demand + LLM-suggested** (triggers on explicit planning/design requests, or when the LLM flags an ADR-worthy conversation and the user confirms) **Path resolution**: if this repo is the canonical protocol host (see `@repo` block below — typically AyCode.Core), the paths are local: `.github/skills//SKILL.md`. Otherwise, prefix with this repo's `own-dep-repos` AyCode.Core path (see the `## Shared Agent Skills` section below for explicit paths). @@ -72,7 +73,7 @@ You are operating in a multi-repo, documentation-first architecture. You MUST ST **Amortization — critical, do NOT re-evaluate per-turn**: the Session Setup cost is measured over the ENTIRE session, not per single turn. A typical session has many turns; the first domain question alone already recoups the investment (alternative — repeated source-code `Grep`/`Read` per turn — costs 10-20K tokens *per turn* with lower output quality). Do NOT flag pre-loaded content as "wasteful" for turns that don't invoke it — the design depends on cross-turn amortization + Rule #3 (no-re-read) + on-demand specificity of Rule #4 (Context Recovery). This amortization is the **central token-economics principle** of the entire protocol stack. -The first response's `[LOADED_DOCS: ...]` prefix must reflect **4 files** (this `copilot-instructions.md` + 3 SKILL.md). +The first response's `[LOADED_DOCS: ...]` prefix must reflect **5 files** (this `copilot-instructions.md` + 4 SKILL.md). ## Documentation-first coding diff --git a/.github/skills/adr-author/SKILL.md b/.github/skills/adr-author/SKILL.md new file mode 100644 index 0000000..5e46e1c --- /dev/null +++ b/.github/skills/adr-author/SKILL.md @@ -0,0 +1,170 @@ +--- +name: adr-author +description: Create Architecture Decision Records (ADRs) for architecturally significant design decisions. Invoke when the user explicitly requests planning/design work ("let's plan X", "decide Y vs Z", "design the W module", "create an ADR for ..."), OR when the conversation drifts into architecturally significant territory and you proactively flag it (user must confirm — never auto-invoke). Handles two paradigms: per-repo `docs/adr/NNNN-.md` files (Nygard style, product/code decisions) and the workspace-level `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` table row (LLMP-DEC-N, protocol-meta decisions). Structured interview: context → alternatives → trade-offs → decision → consequences. Always produces a durable `.md` artifact, never just a conversational answer. +compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Read/Write/Edit/Glob/Grep tools. +metadata: + author: Fullepi + version: "1.0" +--- + +# adr-author + +Structured interview-style skill that captures architecturally significant decisions as Architecture Decision Records. The output is a durable artifact (a new `.md` file or a new table row) — the decision, its context, the alternatives considered, and its consequences remain accessible months or years later. Without this skill, design rationale tends to disappear into chat history or get flattened into TODO entries that lose the "why". + +## When to invoke + +### Explicit user triggers +- "tervezzük meg X", "let's plan X", "design the W module" +- "döntsük el Y vagy Z", "decide between Y and Z" +- "válasszunk ", "choose an approach to V" +- "milyen megközelítést", "what's our approach to W" +- "create an ADR for …", "write a decision record about …" + +### LLM-suggest-back (you flag, user confirms) + +When the current conversation exhibits **two or more** of these heuristics, flag the user: + +- Multiple alternatives being actively weighed +- Decision that is irreversible or expensive to reverse +- Cross-cutting impact (touches 2+ modules, or framework↔consumer boundary) +- A question someone might reasonably re-open in 6-18 months and ask "wait, why did we pick this?" +- Non-trivial dependency or library choice +- New public-facing API shape being discussed + +Example flag phrasing: + +> "Ez egy architektúra-szintű választásnak tűnik (több alternatíva, irreverzibilis lehet). Érdemes lenne ADR-formába önteni — invokáljam az `adr-author` skill-t? (Jelezd, ha inkább folytassuk simán válaszként.)" + +**User confirmation is required** before proceeding to Step 1. Never auto-invoke. + +### When NOT to invoke +- Trivial decisions: variable names, local refactors, formatting +- "How does X work" questions → `docs-discovery` territory +- Concrete implementation details (API signature specifics, SQL column types, exact loop structure) → these belong in code comments or the relevant topic doc +- Already-decided topics covered by an existing ADR → dedupe: link to the existing ADR; optionally propose a supersession if the user wants to revisit +- Simple bug fixes → `docs-check` surfaces these as `_ISSUES.md` entries + +## Step 1 — Routing: which decision log? + +Two paradigms coexist in the workspace: + +| Paradigm | Location | Shape | Used for | +|---|---|---|---| +| **Per-repo ADR** (Nygard style) | `/docs/adr/NNNN-.md` | One file per decision | Product / code / domain architecture decisions | +| **Decision log table** | `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` | Table row with ID `LLMP-DEC-N` | Protocol-meta: rule changes, skill additions, instruction-file structural shifts | + +Heuristic: +- Topic concerns `.github/`, `copilot-instructions.md`, skills, `[LOADED_DOCS]` format, Rule #1-5 → **LLMP table row** +- Topic concerns code, domain, architecture, library choice, API shape, infrastructure → **per-repo ADR file** + +If ambiguous, ask the user explicitly before proceeding. + +**Path resolution for `docs/adr/`** — most ADRs are written for a new feature, refactor, or design decision in an established codebase where the convention is already settled. Don't overthink placement. + +**Common case (most ADRs)**: run `Glob /**/docs/adr/*.md`. If matches exist, use that established folder — convention is settled, no placement decision needed. **Never create a parallel `docs/adr/` elsewhere** in the same repo; fragmenting defeats sequential numbering, cross-references, and audit-ability. + +Placement IS a real decision only when no `docs/adr/` exists yet in the relevant scope. Two sub-cases: + +- **Fresh repo / first ADR ever**: default to `/docs/adr/` at the repo root. Propose creating it with `README.md` (2-3 lines explaining the convention) and `0000-TEMPLATE.md` (copy of `references/ADR_TEMPLATE.md`). User confirms (Rule #5) before creation. + +- **Multi-project repo, no `docs/adr/` anywhere yet**: match scope to placement — + - **Cross-cutting decision** (affects 2+ projects, framework↔consumer boundary, or repo-wide concerns such as auth, logging conventions, build infrastructure) → place at the **highest common ancestor** of affected scopes, typically `/docs/adr/` at the repo root. + - **Project-scoped decision** (one project's internal architecture only) → `//docs/adr/`. + - **Ambiguous scope** → ask the user explicitly. Suggested phrasing: *"this decision touches projects X and Y — should the ADR live at the repo-root `docs/adr/` (cross-cutting) or scoped to one of those projects?"* + +The chosen location persists in git history and inbound cross-references (other ADRs, code comments, docs); relocating later requires updating every cross-ref. Getting placement right at creation is cheaper than fixing it. + +## Step 2 — Establish context (Socratic) + +Ask, one at a time or in a short batch: + +1. **Problem/opportunity** — what's the trigger? A symptom, constraint, or new requirement? +2. **Timing** — why decide NOW? (If "we don't actually need to decide yet" is the honest answer, exit cleanly and suggest a `_TODO.md` entry instead.) +3. **Scope boundaries** — what does this decision affect? What is explicitly OUT of scope? + +If the user answers "I don't know" to most of these, the decision isn't ripe. Gently suggest: explore the problem first, revisit when it's sharper. Exit skill. + +## Step 3 — Surface alternatives + +Actively ask: "What options are you considering?" + +Then YOU add alternatives the user didn't mention — draw from loaded docs, known patterns, framework conventions. Aim for **minimum 2, ideally 3+** alternatives. If only 1 option is viable after honest thought, it's not a decision — it's an implementation path. Exit and suggest a `_TODO.md` entry. + +**Guardrail**: don't invent strawmen. Every alternative listed must be a genuinely considered option, not an obviously-bad filler to make the decision look rigorous. + +## Step 4 — Trade-off elicitation + +For each alternative, capture: +- **Pros** — what it enables / is good at +- **Cons** — what it sacrifices / breaks +- **Reversibility** — how expensive is it to undo later? +- **Cost** — implementation time, complexity, operational burden +- **Future flexibility** — does it close doors on future options? + +Keep it concise: 2-4 bullets per category per alternative. Not a dissertation. + +## Step 5 — Decision capture + +**The user chooses. You never decide.** Your job is to surface, weigh, and ask clarifying questions. After the user commits: + +- Confident → `Status: Accepted (YYYY-MM-DD)` +- Tentative/experimental → `Status: Proposed (YYYY-MM-DD)` — revisit later +- Time-boxed → `Status: Accepted, review by YYYY-MM-DD` + +## Step 6 — Consequences projection + +With loaded docs + the chosen decision, list consequences: +- **Positive** — what this unlocks / improves +- **Negative** — what it costs / breaks / constrains +- **Follow-ups required** — "this decision requires updating X, Y, Z" + +Be concrete: reference specific files, types, or other docs when relevant. Vague consequences ("might affect performance") are less useful than specific ones ("adds ~200ms latency to the `/orders` endpoint under 10K+ items — see `PERFORMANCE.md`"). + +Follow-ups typically become either `_TODO.md` entries or new ADRs in their own right (if the follow-up is itself a significant sub-decision). + +## Step 7 — Draft, review, write + +Assemble the draft using `references/ADR_TEMPLATE.md`. Show the draft to the user. The user reviews, requests changes; iterate as needed. + +**Only with explicit user approval** (Rule #5) write the file: + +### Per-repo ADR write +- Path: `/NNNN-.md` — `` is the location resolved in Step 1 (single-project repos default to `/docs/adr/`; multi-project repos follow the placement rules in Step 1's "Multi-project repo case"). +- NNNN: zero-padded 4 digits (e.g., `0001`, `0042`). Derive from `Glob /*.md` and take max+1. +- ``: short kebab-case derived from the title (e.g., `cursor-based-pagination`, `postgres-over-mongo`). + +### LLMP decision log write +- File: `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` +- Append one table row to the `## 2026` (or current year) section. +- ID: derive the next `LLMP-DEC-N` from the max existing in the file + 1. +- Condense the full ADR body into 4 columns: Decision (one line), Rationale (1-3 sentences), Affected (scope codes — see the file's own "Scope codes" section). + +## Step 8 — Cross-references and follow-ups + +After the primary write: + +1. **Quick-reference update** — if the repo has a `PRODUCT_DECISIONS.md` (or equivalent ADR index), append a new row. +2. **Supersession chain** — if this ADR supersedes an earlier one, edit the older ADR's `Status` line: `Superseded by ADR-NNNN (YYYY-MM-DD)`. +3. **Issue/TODO resolution** — if this decision closes an open entry in `{TOPIC}_ISSUES.md` or `{TOPIC}_TODO.md`, propose updating those entries with `Status: RESOLVED by ADR-NNNN`. Do not apply without user consent. +4. **Follow-up TODOs** — if Step 6 surfaced follow-up work items, propose their addition to the relevant `{TOPIC}_TODO.md` as new `{TOPIC}-T-N` entries. Do not apply without user consent. + +## Handoff with other skills + +- **`docs-check`** — when docs-check (at the end of a code-modifying response) notices an observation that looks ADR-worthy (irreversible decision landed in code; alternatives were actively weighed in the session; cross-cutting impact), it SHOULD flag: *"this may warrant an ADR — consider invoking `adr-author`"*. Does not auto-invoke. +- **`docs-discovery`** — if the user's question is "how does X work" rather than "let's design X", `docs-discovery` owns the response, not this skill. +- **`protocol-audit`** — unrelated; audits file-structural consistency, doesn't interact with ADR content. + +## Tool usage + +Tool-neutral. Map to the host agent's tools: +- Reading existing ADRs: `Read`, `Glob /docs/adr/*.md` +- Writing new ADR: `Write` (new file) or `Edit` (append LLMP table row) +- Dedupe check: `Grep` on existing ADR bodies for the topic keyword + +## Edge cases + +- **Parallel LLM sessions / numbering collision**: derive NNNN at the moment of write, not earlier. If two ADRs end up with the same number, the later writer re-numbers and updates cross-refs. Rare; manual resolution. +- **User changes mind mid-interview**: if the user indicates "actually we don't need to decide this yet", exit cleanly. Offer to write a `_TODO.md` entry instead so the question isn't lost. +- **Retroactive ADR for a decision already made in code**: legitimate. Document it honestly: `Status: Accepted (retroactive, YYYY-MM-DD), originally decided ~`. The value is the rationale preservation, even after-the-fact. +- **Decision spans multiple repos**: write the primary ADR in the repo that owns the change most-centrally; add short `See /docs/adr/NNNN-.md` cross-refs in the other affected repos' ADR folders (under the same NNNN if possible; otherwise a free slot in each). +- **User proposes an ADR for something trivial**: politely suggest `_TODO.md` instead. Don't lecture — just offer the lighter alternative once; if they still want an ADR, proceed. diff --git a/.github/skills/adr-author/references/ADR_TEMPLATE.md b/.github/skills/adr-author/references/ADR_TEMPLATE.md new file mode 100644 index 0000000..c109ea8 --- /dev/null +++ b/.github/skills/adr-author/references/ADR_TEMPLATE.md @@ -0,0 +1,68 @@ +# ADR NNNN: + + + +## Status + + + +Proposed + +## Context + + + +## Decision + + + +## Consequences + +**Positive:** +- +- <...> + +**Negative:** +- +- <...> + +**Follow-ups required:** +- +- <...> + +## Alternatives considered + + + +- **** (rejected): +- **** (rejected): + +## Related + + + +- Supersedes: +- Superseded by: +- Related ADRs: +- Related TODOs/Issues: +- External references: diff --git a/AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md b/AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md index 83ed985..aafb272 100644 --- a/AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md +++ b/AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md @@ -172,6 +172,56 @@ See `LOGGING_TODO.md#log-t-11`. - Sibling gap: `../SIGNALR/SIGNALR_ISSUES.md#sig-i-7` (same server-side plugin, protocol-options adoption gap) - Plugin doc drift: `Nop.Plugin.Misc.AIPlugin/docs/SIGNALR/README.md:22` still documents the pre-migration `new AcBinaryHubProtocol()` registration (actual code uses `.AddAcBinaryProtocol(opts => {...})`). Update needed. +## LOG-I-9: JWT signing key written to log (CRITICAL security) + +**Severity:** 🛑 **Critical (security)** · **Status:** Open · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192` + +### Description +`GenerateAccessToken` writes the JWT signing key to the logger via `GlobalLogger.Detail`: + +```csharp +GlobalLogger.Detail($"Key: {configuration["JWT:Key"]}"); +``` + +The signing key is the cryptographic secret used to sign all JWT access tokens. Anyone with read access to the logs (file system, log aggregators, retention archives, operator dashboards, screenshots during incident response) can: + +1. Forge arbitrary tokens for any user +2. Impersonate any role / claim +3. Bypass authentication entirely + +Log storage is typically at a lower trust level than dedicated secret stores. This exposure path is a complete authentication bypass. + +### Fix direction +Remove the `GlobalLogger.Detail($"Key: ...")` line entirely. The signing key must NEVER appear in any log output. If presence verification is needed for diagnostics, log only a hash prefix (e.g. `Key configured: SHA256[0..8] = abc12345`). + +### Related +- `LOG-I-10` (sibling — access token also leaked, same file, same code path) +- `LOGGING/README.md` may need a new "Never log secrets" guideline section (separate `_TODO.md` candidate) +- **Discovery context**: this issue and `LOG-I-10` emerged during the user-bearer-token ADR context-gathering phase (2026-04-25 Copilot session, while reading `AcLoginServiceServer.cs`). If a dedicated AUTH topic-folder is created later (likely as part of the bearer token ADR follow-ups), these two entries are candidates for relocation to `AUTH_ISSUES.md` with `Status: SUPERSEDED by AUTH-I-N` cross-references. + +## LOG-I-10: JWT access token written to log (CRITICAL security) + +**Severity:** 🛑 **Critical (security)** · **Status:** Open · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212` + +### Description +After `GenerateAccessToken` produces a token, the token text itself is written to the logger: + +```csharp +GlobalLogger.Detail($"AccesToken: {writtenToken}"); +``` + +(Side note: `AccesToken` is a typo for `AccessToken` — fix while at the line.) + +Access tokens are bearer credentials — anyone holding the token can authenticate as the user until token expiry (currently 6h per `GenerateAccessToken` config). Logged tokens leak via the same channels as `LOG-I-9` (live streams, retention archives, screenshots, shared logs during ops). + +### Fix direction +Remove the `GlobalLogger.Detail($"AccesToken: ...")` line entirely. If issuance verification is needed, log only **metadata** — user ID, expiry timestamp, issuer — never the token itself. + +### Related +- `LOG-I-9` (sibling — JWT signing key also leaked, same file) +- Same `LOGGING/README.md` "Never log secrets" guideline gap as LOG-I-9 +- Same discovery-context note as LOG-I-9 (bearer token ADR session, candidate for AUTH topic relocation if/when that emerges) + ## Evaluated review findings — NOT bugs (by-design) Cross-session review (2026-04) flagged several items that are **intentional defense-in-depth / YAGNI** and should NOT be "fixed":