[LOADED_DOCS: 3 files, no new loads]
ADR skill, status vocab, and security log fixes - Enhanced `adr-author` skill and ADR template based on first use: clarified opinion sharing, cross-referencing, index updates, and parallel session handling; rewrote ADR template pointer and added first repo-wide ADR. - Updated skill loading: only `docs-discovery` and `docs-check` pre-loaded; others lazy-loaded; removed `version` from SKILL.md frontmatters. - Simplified issue/TODO status to `Open`, `InProgress`, `Closed`; updated archiving logic and migrated all entries. - Added forward-slash glob pattern rule to `docs-discovery/SKILL.md`. - Closed LOG-I-9 and LOG-I-10: DEBUG-gated sensitive log lines, fixed typo, and cross-referenced new ADR. - Various documentation and index improvements.
This commit is contained in:
parent
9a53aa1d73
commit
8f72593741
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
|
|
@ -4,7 +4,6 @@ description: Create Architecture Decision Records (ADRs) for architecturally sig
|
||||||
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Read/Write/Edit/Glob/Grep tools.
|
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Read/Write/Edit/Glob/Grep tools.
|
||||||
metadata:
|
metadata:
|
||||||
author: Fullepi
|
author: Fullepi
|
||||||
version: "1.0"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# adr-author
|
# adr-author
|
||||||
|
|
@ -105,7 +104,14 @@ Keep it concise: 2-4 bullets per category per alternative. Not a dissertation.
|
||||||
|
|
||||||
## Step 5 — Decision capture
|
## Step 5 — Decision capture
|
||||||
|
|
||||||
**The user chooses. You never decide.** Your job is to surface, weigh, and ask clarifying questions. After the user commits:
|
**The user chooses. You never decide.** Your job is to surface, weigh, and ask clarifying questions.
|
||||||
|
|
||||||
|
**When the user explicitly asks for your opinion** ("mi a véleményed?", "what do you think?", "which would you pick?"): share your weighing — but frame it as preference, not prescription. End with the user's decision still open.
|
||||||
|
|
||||||
|
- ❌ Avoid: *"X is obviously the right choice."* / *"Path: repo-root, egyértelmű."*
|
||||||
|
- ✅ Prefer: *"I lean toward X because Y, but you decide."*
|
||||||
|
|
||||||
|
After the user commits:
|
||||||
|
|
||||||
- Confident → `Status: Accepted (YYYY-MM-DD)`
|
- Confident → `Status: Accepted (YYYY-MM-DD)`
|
||||||
- Tentative/experimental → `Status: Proposed (YYYY-MM-DD)` — revisit later
|
- Tentative/experimental → `Status: Proposed (YYYY-MM-DD)` — revisit later
|
||||||
|
|
@ -130,7 +136,7 @@ Assemble the draft using `references/ADR_TEMPLATE.md`. Show the draft to the use
|
||||||
|
|
||||||
### Per-repo ADR write
|
### Per-repo ADR write
|
||||||
- Path: `<adr-folder>/NNNN-<slug>.md` — `<adr-folder>` is the location resolved in Step 1 (single-project repos default to `<repo>/docs/adr/`; multi-project repos follow the placement rules in Step 1's "Multi-project repo case").
|
- Path: `<adr-folder>/NNNN-<slug>.md` — `<adr-folder>` is the location resolved in Step 1 (single-project repos default to `<repo>/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 <adr-folder>/*.md` and take max+1.
|
- NNNN: zero-padded 4 digits (e.g., `0001`, `0042`). **Re-Glob `<adr-folder>/*.md` immediately before the `Write` call** (not at draft time) — parallel sessions or interim work may have added entries; deriving NNNN at draft time risks collision.
|
||||||
- `<slug>`: short kebab-case derived from the title (e.g., `cursor-based-pagination`, `postgres-over-mongo`).
|
- `<slug>`: short kebab-case derived from the title (e.g., `cursor-based-pagination`, `postgres-over-mongo`).
|
||||||
|
|
||||||
### LLMP decision log write
|
### LLMP decision log write
|
||||||
|
|
@ -143,11 +149,23 @@ Assemble the draft using `references/ADR_TEMPLATE.md`. Show the draft to the use
|
||||||
|
|
||||||
After the primary write:
|
After the primary write:
|
||||||
|
|
||||||
1. **Quick-reference update** — if the repo has a `PRODUCT_DECISIONS.md` (or equivalent ADR index), append a new row.
|
1. **Quick-reference / index update** — append a new row to whichever ADR-index file exists in the repo. Most commonly: `<adr-folder>/README.md` (which should have an Index table). Other patterns: `PRODUCT_DECISIONS.md` at the repo root, or none. If no index exists, propose creating one in `<adr-folder>/README.md` as part of the same write.
|
||||||
2. **Supersession chain** — if this ADR supersedes an earlier one, edit the older ADR's `Status` line: `Superseded by ADR-NNNN (YYYY-MM-DD)`.
|
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.
|
3. **Issue/TODO resolution** — two cases:
|
||||||
|
- **Decision-closes-entry**: if this ADR's decision resolves an `Open` entry in `{TOPIC}_ISSUES.md` or `{TOPIC}_TODO.md`, propose updating those entries to `Status: Closed (YYYY-MM-DD)` with a `### Resolution` sub-section referencing this ADR (per `TOPIC_CODES.md` Status conventions).
|
||||||
|
- **Pre-flight-closed-entry**: if an entry was closed BEFORE the ADR write (a pre-flight fix that this ADR formalizes architecturally), do NOT change its Status — it's already `Closed`. Instead, propose appending a soft cross-ref to the entry's `### Related` section: `**Reference:** ADR-NNNN — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.`
|
||||||
|
|
||||||
|
Do not apply either case 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.
|
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.
|
||||||
|
|
||||||
|
## Step 9 — Hand-off after write
|
||||||
|
|
||||||
|
After Steps 7 + 8 complete, do not assume what comes next. Ask the user explicitly:
|
||||||
|
|
||||||
|
> "ADR-NNNN written and cross-references updated. What's next? Common options: commit changes / promote `Status: Proposed → Accepted (YYYY-MM-DD)` / start implementation work / open the next ADR for a related decision. Which?"
|
||||||
|
|
||||||
|
Keep it short — let the user steer. If the user already indicated direction in earlier turns ("after this ADR, we'll start implementation"), reflect that in the hand-off rather than enumerating from scratch.
|
||||||
|
|
||||||
## Handoff with other skills
|
## 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-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.
|
||||||
|
|
@ -163,7 +181,7 @@ Tool-neutral. Map to the host agent's tools:
|
||||||
|
|
||||||
## Edge cases
|
## 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.
|
- **Parallel LLM sessions / numbering collision**: covered by Step 7's "Re-Glob immediately before Write" rule. If two ADRs nonetheless end up with the same NNNN (concurrent writes within the glob-to-write window), 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.
|
- **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 ~<when>`. The value is the rationale preservation, even after-the-fact.
|
- **Retroactive ADR for a decision already made in code**: legitimate. Document it honestly: `Status: Accepted (retroactive, YYYY-MM-DD), originally decided ~<when>`. 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 <repo>/docs/adr/NNNN-<slug>.md` cross-refs in the other affected repos' ADR folders (under the same NNNN if possible; otherwise a free slot in each).
|
- **Decision spans multiple repos**: write the primary ADR in the repo that owns the change most-centrally; add short `See <repo>/docs/adr/NNNN-<slug>.md` cross-refs in the other affected repos' ADR folders (under the same NNNN if possible; otherwise a free slot in each).
|
||||||
|
|
|
||||||
|
|
@ -11,9 +11,15 @@
|
||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
<!-- One of: Proposed | Accepted (YYYY-MM-DD) | Superseded by ADR-XXXX (YYYY-MM-DD) | Rejected (YYYY-MM-DD) -->
|
<!--
|
||||||
|
One of: Proposed (YYYY-MM-DD) | Accepted (YYYY-MM-DD) | Superseded by ADR-XXXX (YYYY-MM-DD) | Rejected (YYYY-MM-DD)
|
||||||
|
|
||||||
Proposed
|
Note: ADR Status uses Nygard's classic 4-value vocabulary, distinct from
|
||||||
|
`_ISSUES.md` / `_TODO.md` 3-value vocabulary (Open / InProgress / Closed).
|
||||||
|
See `docs-check/references/TOPIC_CODES.md` "Status field conventions" for the latter.
|
||||||
|
-->
|
||||||
|
|
||||||
|
Proposed (YYYY-MM-DD)
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
|
|
@ -52,9 +58,17 @@ Proposed
|
||||||
<!--
|
<!--
|
||||||
Minimum 2 alternatives. If you only had 1 "option", it wasn't a decision —
|
Minimum 2 alternatives. If you only had 1 "option", it wasn't a decision —
|
||||||
it was an implementation path. Consider whether this file is the right artifact.
|
it was an implementation path. Consider whether this file is the right artifact.
|
||||||
|
|
||||||
|
For "obviously bad" rejections, the one-line reason is enough. Only add
|
||||||
|
the dimension sub-bullets (Reversibility / Cost / Future flexibility) when
|
||||||
|
alternatives differ MATERIALLY on those dimensions — don't pad every
|
||||||
|
rejection with all four dimensions.
|
||||||
-->
|
-->
|
||||||
|
|
||||||
- **<alternative name>** (rejected): <one-sentence reason>
|
- **<alternative name>** (rejected): <one-sentence reason>
|
||||||
|
- *Reversibility:* <only if cost-of-undo materially differs>
|
||||||
|
- *Cost:* <only if implementation cost materially differs>
|
||||||
|
- *Future flexibility:* <only if it closes doors on later options>
|
||||||
- **<alternative name>** (rejected): <one-sentence reason>
|
- **<alternative name>** (rejected): <one-sentence reason>
|
||||||
|
|
||||||
## Related
|
## Related
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,6 @@ description: Rotate closed entries (Status: Fixed/Resolved/Won't fix/Superseded
|
||||||
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Read/Write/Edit/Glob/Grep tools.
|
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Read/Write/Edit/Glob/Grep tools.
|
||||||
metadata:
|
metadata:
|
||||||
author: Fullepi
|
author: Fullepi
|
||||||
version: "1.0"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# docs-archive
|
# docs-archive
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,6 @@ description: Evaluate loaded `.md` documentation at the end of every code-modify
|
||||||
compatibility: Designed for Claude Code and GitHub Copilot. Invoked at the end of code-modifying responses. Uses only content already in LOADED_DOCS.
|
compatibility: Designed for Claude Code and GitHub Copilot. Invoked at the end of code-modifying responses. Uses only content already in LOADED_DOCS.
|
||||||
metadata:
|
metadata:
|
||||||
author: Fullepi
|
author: Fullepi
|
||||||
version: "1.0"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# docs-check
|
# docs-check
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,6 @@ description: Load all .md documentation relevant to the user's current coding ta
|
||||||
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Glob/Read tools.
|
compatibility: Designed for Claude Code and GitHub Copilot (VS). Uses the host agent's Glob/Read tools.
|
||||||
metadata:
|
metadata:
|
||||||
author: Fullepi
|
author: Fullepi
|
||||||
version: "1.0"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# docs-discovery
|
# docs-discovery
|
||||||
|
|
@ -54,6 +53,19 @@ The `**/` is NOT cosmetic. It matches `docs/` at **any depth** in the workspace:
|
||||||
|
|
||||||
**The rule is absolute**: NEVER drop the leading `**/`, even when you "know" the repo. Let the recursive glob find the actual depth. Relative-path guesses based on "usual" layouts are a reliable source of false-empty conclusions.
|
**The rule is absolute**: NEVER drop the leading `**/`, even when you "know" the repo. Let the recursive glob find the actual depth. Relative-path guesses based on "usual" layouts are a reliable source of false-empty conclusions.
|
||||||
|
|
||||||
|
### ⚠️ Path separators — forward slashes ONLY in glob patterns
|
||||||
|
|
||||||
|
Glob engines (ripgrep, `Microsoft.Extensions.FileSystemGlobbing`, Node `glob`, Python `pathlib`, etc.) **all expect forward slashes `/` in patterns**, regardless of host OS. The filesystem layer handles OS-level path normalization; the pattern stays POSIX.
|
||||||
|
|
||||||
|
**Correct (always)**: `H:/Applications/<Repo>/**/docs/{TOKEN}/**/*.md`
|
||||||
|
**Wrong (Windows trap)**: `H:\Applications\<Repo>\**\docs\{TOKEN}\**\*.md`
|
||||||
|
|
||||||
|
**Failure mode**: a Windows-backslash glob typically returns **0 matches even when files exist** — silent failure. The agent sees "no docs", falls through to code-search, and never realizes the search was malformed.
|
||||||
|
|
||||||
|
**Rule is absolute**: write `/` in every glob pattern. If your tool/runtime is Windows and you're tempted to "match the OS path style" — don't. The glob engine will not normalize backslashes for you in pattern position.
|
||||||
|
|
||||||
|
(Combined with the `**/` mandate above: every glob in this skill uses `<optional-prefix>/**/<pattern>` with forward slashes throughout — both rules together cover the most common false-empty conclusions.)
|
||||||
|
|
||||||
### File layout convention
|
### File layout convention
|
||||||
|
|
||||||
(See `LLM_PROTOCOL_DECISIONS.md` entry "Docs migrated to folder+README pattern".)
|
(See `LLM_PROTOCOL_DECISIONS.md` entry "Docs migrated to folder+README pattern".)
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,6 @@ description: Audit all `.github/copilot-instructions.md` files registered in `re
|
||||||
compatibility: Designed for Claude Code and GitHub Copilot (VS). Requires read access to the paths listed in `references/REPOS.md`.
|
compatibility: Designed for Claude Code and GitHub Copilot (VS). Requires read access to the paths listed in `references/REPOS.md`.
|
||||||
metadata:
|
metadata:
|
||||||
author: Fullepi
|
author: Fullepi
|
||||||
version: "2.2"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# Protocol Audit
|
# Protocol Audit
|
||||||
|
|
|
||||||
|
|
@ -174,7 +174,7 @@ See `LOGGING_TODO.md#log-t-11`.
|
||||||
|
|
||||||
## LOG-I-9: JWT signing key written to log (CRITICAL security)
|
## LOG-I-9: JWT signing key written to log (CRITICAL security)
|
||||||
|
|
||||||
**Severity:** 🛑 **Critical (security)** · **Status:** Open · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192`
|
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192`
|
||||||
|
|
||||||
### Description
|
### Description
|
||||||
`GenerateAccessToken` writes the JWT signing key to the logger via `GlobalLogger.Detail`:
|
`GenerateAccessToken` writes the JWT signing key to the logger via `GlobalLogger.Detail`:
|
||||||
|
|
@ -194,14 +194,25 @@ Log storage is typically at a lower trust level than dedicated secret stores. Th
|
||||||
### Fix direction
|
### 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`).
|
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`).
|
||||||
|
|
||||||
|
### Resolution
|
||||||
|
|
||||||
|
**What:** Wrapped the `GlobalLogger.Detail($"Key: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block explaining the risk. Line preserved (not deleted) — user opted for DEBUG-gating over removal so local auth-debug retains the diagnostic.
|
||||||
|
|
||||||
|
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:192` (line numbers shifted slightly due to the inserted comment).
|
||||||
|
|
||||||
|
**Why:** User-directed deviation from the originally proposed "remove entirely" fix direction (bearer-token ADR planning session, 2026-04-25). DEBUG-gating ensures production (`-c Release`) builds strip the line at compile time, while preserving dev-time diagnostics.
|
||||||
|
|
||||||
|
**Caveat:** DEBUG builds still write the key to log. CI/CD must build with `-c Release` for production deployments. The inline `// SECURITY:` comment in the code warns future maintainers.
|
||||||
|
|
||||||
### Related
|
### Related
|
||||||
|
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
|
||||||
- `LOG-I-10` (sibling — access token also leaked, same file, same code path)
|
- `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)
|
- `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.
|
- **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)
|
## LOG-I-10: JWT access token written to log (CRITICAL security)
|
||||||
|
|
||||||
**Severity:** 🛑 **Critical (security)** · **Status:** Open · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212`
|
**Severity:** 🛑 **Critical (security)** · **Status:** Closed (2026-04-25) · **Area:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212`
|
||||||
|
|
||||||
### Description
|
### Description
|
||||||
After `GenerateAccessToken` produces a token, the token text itself is written to the logger:
|
After `GenerateAccessToken` produces a token, the token text itself is written to the logger:
|
||||||
|
|
@ -217,7 +228,20 @@ Access tokens are bearer credentials — anyone holding the token can authentica
|
||||||
### Fix direction
|
### 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.
|
Remove the `GlobalLogger.Detail($"AccesToken: ...")` line entirely. If issuance verification is needed, log only **metadata** — user ID, expiry timestamp, issuer — never the token itself.
|
||||||
|
|
||||||
|
### Resolution
|
||||||
|
|
||||||
|
**What:** Two changes:
|
||||||
|
1. Wrapped the `GlobalLogger.Detail($"AccessToken: ...")` call in `#if DEBUG` / `#endif` with a `// SECURITY:` comment block.
|
||||||
|
2. Fixed the `AccesToken` → `AccessToken` typo (flagged in original Description as a side-note) in the log-message string literal.
|
||||||
|
|
||||||
|
**Where:** `AyCode.Services.Server/Logins/AcLoginServiceServer.cs:212` (line numbers shifted slightly due to the inserted comment).
|
||||||
|
|
||||||
|
**Why:** Same as `LOG-I-9` — user opted for DEBUG-gating over deletion (2026-04-25, bearer-token ADR session). Typo fix is opportunistic since the line was being touched.
|
||||||
|
|
||||||
|
**Caveat:** Same DEBUG-build leak constraint as `LOG-I-9`. CI/CD must build with `-c Release` for production.
|
||||||
|
|
||||||
### Related
|
### Related
|
||||||
|
- **Reference:** ADR [`0001-user-bearer-token-flow.md`](../../../docs/adr/0001-user-bearer-token-flow.md) — formal architectural decision; this entry's Resolution is the pre-flight fix referenced in the ADR's Consequences.
|
||||||
- `LOG-I-9` (sibling — JWT signing key also leaked, same file)
|
- `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 `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)
|
- Same discovery-context note as LOG-I-9 (bearer token ADR session, candidate for AUTH topic relocation if/when that emerges)
|
||||||
|
|
|
||||||
|
|
@ -224,7 +224,7 @@ public class AcLoginServiceServer<TResultLoggedInModel, TDal, TDbContext, TUser,
|
||||||
// bearer credential. Production build-ből kötelezően kimarad
|
// bearer credential. Production build-ből kötelezően kimarad
|
||||||
// (#if DEBUG). NEM törölve, mert helyi auth-debug során hasznos.
|
// (#if DEBUG). NEM törölve, mert helyi auth-debug során hasznos.
|
||||||
#if DEBUG
|
#if DEBUG
|
||||||
GlobalLogger.Detail($"AccesToken: {writtenToken}");
|
GlobalLogger.Detail($"AccessToken: {writtenToken}");
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
return writtenToken;
|
return writtenToken;
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,5 @@
|
||||||
|
# ADR Template
|
||||||
|
|
||||||
|
The canonical template lives at [`AyCode.Core/.github/skills/adr-author/references/ADR_TEMPLATE.md`](../../.github/skills/adr-author/references/ADR_TEMPLATE.md) — that's what the `adr-author` skill copies when authoring a new ADR.
|
||||||
|
|
||||||
|
This file exists only as a placeholder so `docs/adr/` numbering starts visibly at `0001`. Don't edit this file; edit the canonical template if the format needs to change.
|
||||||
|
|
@ -0,0 +1,109 @@
|
||||||
|
# ADR 0001: User bearer-token authentication flow (HTTP + SignalR + tag dispatch)
|
||||||
|
|
||||||
|
## Status
|
||||||
|
|
||||||
|
Proposed (2026-04-25)
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
The framework has partial JWT infrastructure on the server side (`AcLoginServiceServer` produces access + refresh tokens via standard `JwtSecurityTokenHandler`; `IAcLoggedInModelBase.AccessToken` returns the token to the client) but no end-to-end bearer-token flow:
|
||||||
|
|
||||||
|
- Client-side login service is not implemented (all methods throw `NotImplementedException`).
|
||||||
|
- HTTP transport does not inject the bearer header on outgoing calls.
|
||||||
|
- SignalR transport has no access-token provider wired into the hub-connection builder.
|
||||||
|
- Server has no `AddJwtBearer` registration, no `[Authorize]` plumbing, and no per-tag authorization for the single-method `OnReceiveMessage(int tag, ...)` SignalR dispatch.
|
||||||
|
|
||||||
|
Two security defects in `AcLoginServiceServer.GenerateAccessToken` (JWT signing key + issued access token written to logger) were addressed pre-flight as `LOG-I-9` and `LOG-I-10` `Closed`. Their existence is the proximate trigger for this ADR — they exposed the absence of an end-to-end auth contract.
|
||||||
|
|
||||||
|
Consumer apps (MAUI + WASM) cannot adopt the framework safely until this ADR's decisions are implemented.
|
||||||
|
|
||||||
|
**Cross-project scope:** AyCode.Services + AyCode.Services.Server + AyCode.Interfaces + AyCode.Models.Server + every consumer. ADR placed at repo-root `docs/adr/` per the multi-project routing rule in [`.github/skills/adr-author/SKILL.md`](../../.github/skills/adr-author/SKILL.md) Step 1 ("cross-cutting decision → highest common ancestor").
|
||||||
|
|
||||||
|
**In scope:** HTTP transport, SignalR transport, per-tag SignalR dispatch authorization, both client (MAUI + WASM) and server.
|
||||||
|
|
||||||
|
**Out of scope (deferred to future ADRs or TODO):**
|
||||||
|
- Refresh-token flow design (sliding lifetime, proactive vs reactive refresh, server-side token store) — separate ADR `0002`.
|
||||||
|
- Role-based / claims-based authorization beyond authenticated / anonymous.
|
||||||
|
- Multi-tenant claim shape.
|
||||||
|
- OAuth2 / external-IdP integration.
|
||||||
|
- Token revocation / blacklist.
|
||||||
|
- Logout server-side invalidation (currently client-side discard only).
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
Adopt a layered bearer-token authentication architecture across all client and server entry points. The architectural choices:
|
||||||
|
|
||||||
|
1. **Token storage is abstracted at the framework layer.** Consumers supply platform-specific implementations (MAUI: secure storage, WASM: protected browser storage). Framework provides the contract and a default in-memory implementation for tests / fallback.
|
||||||
|
|
||||||
|
2. **HTTP transport binding is automatic.** A framework-provided delegating handler reads from the token store and injects the bearer header on every outgoing call. Consumers register one named HttpClient configuration; per-call header injection is eliminated.
|
||||||
|
|
||||||
|
3. **SignalR transport binding is automatic.** A framework-provided hub-builder extension wires the access-token provider from the token store. Browser-hosted clients (WASM) use the standard query-string token-transport fallback because the WebSocket API cannot carry custom headers; server accepts both header and query-string forms transparently for `/hub`-prefixed paths.
|
||||||
|
|
||||||
|
4. **Server validates JWTs via the standard ASP.NET Core authentication pipeline.** A framework DI bundle binds JWT options from consumer configuration and registers JwtBearer authentication, including hub-aware events that extract the query-string token for SignalR connections.
|
||||||
|
|
||||||
|
5. **Per-tag SignalR dispatch is gated by an authorization attribute.** The hub class requires authentication by default; specific tags (login, ping, public broadcast) are exempted via an explicit allowlist attribute on the hub. The dispatcher checks authorization before resolving the target method for a given tag, keeping the existing `[Tag(N)]` registry as the single source of truth (no parallel policy registry).
|
||||||
|
|
||||||
|
6. **Token lifetime is 24 hours, regenerated fresh on each login.** Lifetime is configurable per consumer; refresh-flow design is deferred to ADR `0002`. When the access token expires the user re-authenticates (no mid-session refresh in this ADR's scope).
|
||||||
|
|
||||||
|
7. **Security hardening is built into the framework setup.** JWT options are validated at startup (key length ≥ 256 bits for HS256; issuer + audience non-empty); HTTPS metadata is required outside `Development`. Pre-existing log-leak defects were addressed pre-flight (see Consequences → Pre-flight done).
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
**Positive:**
|
||||||
|
- Consumers get bearer-auth via three DI calls (one for JWT bundle, one for HTTP-handler registration, one for hub-builder extension) — minimum-boilerplate target met.
|
||||||
|
- Per-tag authorization scales with the existing tag registry — no parallel registry to maintain.
|
||||||
|
- Two confirmed-critical security leaks (`LOG-I-9`, `LOG-I-10`) are closed; future similar mistakes prevented by the framework guideline doc (`docs/AUTH/README.md`'s "Never log secrets" section).
|
||||||
|
- WASM and MAUI use one shared token-store abstraction; consumer code is platform-aware only at registration time.
|
||||||
|
- Framework-first preserved: all architectural pieces live in AyCode.Services / AyCode.Services.Server / AyCode.Interfaces; consumers supply only platform-storage and config values.
|
||||||
|
|
||||||
|
**Negative:**
|
||||||
|
- 24-hour fixed lifetime forces re-authentication at expiry; user experience friction until ADR `0002` (refresh) lands.
|
||||||
|
- Query-string token in WASM SignalR connections → access logs must redact `access_token` URL parameters (consumer Kestrel/proxy config concern; documented in `docs/AUTH/README.md`).
|
||||||
|
- Existing consumer projects must add the new DI calls and the per-hub allowlist attribute — breaking change for any consumer currently relying on an unauthenticated hub.
|
||||||
|
|
||||||
|
**Follow-ups required:**
|
||||||
|
|
||||||
|
*Pre-flight (already landed in this commit series):*
|
||||||
|
- ✅ `LOG-I-9` closed: JWT signing key log-write DEBUG-gated.
|
||||||
|
- ✅ `LOG-I-10` closed: access token log-write DEBUG-gated; `AccesToken` typo fixed at the same line.
|
||||||
|
|
||||||
|
*Infrastructure (one-time, before implementation):*
|
||||||
|
- Create `docs/AUTH/` topic folder with `README.md` (consumer recipe: class names, method signatures, wire formats, diagrams, code examples), `AUTH_ISSUES.md`, `AUTH_TODO.md`.
|
||||||
|
- Propose new topic code `AUTH` for the workspace registry (`TOPIC_CODES.md` row + Decision Log entry `LLMP-DEC-N`).
|
||||||
|
|
||||||
|
*Implementation (separate commit series, framework layer):*
|
||||||
|
- Token-store abstraction + in-memory default implementation.
|
||||||
|
- HTTP delegating handler + DI extension method bundling its registration with a named HttpClient.
|
||||||
|
- SignalR hub-builder extension (client side) + JWT DI bundle (server side) including hub-aware query-string token events.
|
||||||
|
- Authorization attribute + tag-allowlist attribute + dispatcher hook in the hub base.
|
||||||
|
- JWT options POCO + startup `IValidateOptions<T>` validator.
|
||||||
|
- Repo-wide grep follow-up: search for any other call sites that may log secrets in similar patterns (preventive).
|
||||||
|
|
||||||
|
*Consumer wiring (per-consumer projects, separate tasks):*
|
||||||
|
- MAUI token-store implementation (consumer side).
|
||||||
|
- WASM token-store implementation (consumer side).
|
||||||
|
- Per-consumer `appsettings.json` `AyCode:Jwt` section.
|
||||||
|
- Per-consumer hub: tag allowlist for login + ping (at minimum).
|
||||||
|
|
||||||
|
*Status migration on AUTH topic creation:*
|
||||||
|
- Move `LOG-I-9` and `LOG-I-10` to `AUTH_ISSUES.md` per their existing "Discovery context" notes; in `LOGGING_ISSUES.md` replace bodies with cross-ref `Superseded by AUTH-I-N`.
|
||||||
|
|
||||||
|
*Future ADRs:*
|
||||||
|
- `0002` — Refresh-token flow + sliding-vs-fixed lifetime decision.
|
||||||
|
- Possibly later: role/claims authorization, multi-tenant claims, OAuth2 / external-IdP (per the deferred OUT-of-scope items).
|
||||||
|
|
||||||
|
## Alternatives considered
|
||||||
|
|
||||||
|
- **In-memory-only token store** (rejected): app-restart re-login on MAUI is unacceptable UX.
|
||||||
|
- **Per-call manual `Authorization` header injection** (rejected): boilerplate + error-prone; framework should encapsulate.
|
||||||
|
- **Hub-class-only `[Authorize]` without per-tag allowlist** (rejected): login + ping tags would also require auth → bootstrap deadlock.
|
||||||
|
- **Per-tag DI policy registry** (rejected): too much complexity for Layer 0; mixes consumer-specific tag lists into framework.
|
||||||
|
- **Custom token middleware from scratch** (rejected): non-standard, no `[Authorize]` interop, no out-of-the-box JWT validation.
|
||||||
|
- *Future flexibility:* locks out standard OAuth2 / external-IdP integration when ADR `0002+` extends scope; the chosen `JwtBearer` pipeline keeps that door open.
|
||||||
|
|
||||||
|
## Related
|
||||||
|
|
||||||
|
- Pre-flight fix entries: [`LOGGING_ISSUES.md#log-i-9`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md), [`LOGGING_ISSUES.md#log-i-10`](../../AyCode.Core/docs/LOGGING/LOGGING_ISSUES.md) (both `Closed (2026-04-25)`).
|
||||||
|
- Tentative TODO surfaced post-fix: `LOG-T-12` ("Never log secrets" guideline section in `LOGGING/README.md`).
|
||||||
|
- External: RFC 6750 (Bearer Token Usage), RFC 7519 (JWT), Microsoft.AspNetCore.SignalR auth docs (query-string token pattern).
|
||||||
|
- Future: ADR `0002` (refresh-flow design).
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
# Architecture Decision Records (ADRs)
|
||||||
|
|
||||||
|
Repo-wide Architecture Decision Records — design decisions that affect two or more projects in this repo, framework↔consumer boundaries, or other repo-wide concerns. Project-scoped decisions (internal to a single sub-project) belong in that project's `<project>/docs/adr/` folder, not here.
|
||||||
|
|
||||||
|
See [`.github/skills/adr-author/SKILL.md`](../../.github/skills/adr-author/SKILL.md) for the full authoring procedure (routing rules, Socratic interview, trade-off elicitation, draft → review → write).
|
||||||
|
|
||||||
|
## Convention
|
||||||
|
|
||||||
|
- **Format:** Nygard-style ADR; one file per decision.
|
||||||
|
- **Filename:** `NNNN-<slug>.md` — zero-padded 4-digit sequence + kebab-case slug derived from the title (e.g. `0001-user-bearer-token-flow.md`).
|
||||||
|
- **Numbering:** sequential, append-only. Derive next NNNN from `max(existing) + 1` at write time.
|
||||||
|
- **Template:** copy [`0000-TEMPLATE.md`](0000-TEMPLATE.md) when starting a new ADR.
|
||||||
|
- **Status field:** `Proposed (YYYY-MM-DD)` → `Accepted (YYYY-MM-DD)` / `Rejected (YYYY-MM-DD)` / `Superseded by ADR-XXXX (YYYY-MM-DD)`. Update in-place; entry body / ID / Decision text remain immutable.
|
||||||
|
|
||||||
|
## Index
|
||||||
|
|
||||||
|
| ID | Title | Status |
|
||||||
|
|-------------------------------------------------|------------------------------------------------------------------------|-----------------------|
|
||||||
|
| [0001](0001-user-bearer-token-flow.md) | User bearer-token authentication flow (HTTP + SignalR + tag dispatch) | Proposed (2026-04-25) |
|
||||||
|
|
||||||
|
## Related
|
||||||
|
|
||||||
|
- Protocol-meta decisions (rule changes, skill additions, instruction-file structural shifts) live in [`.github/LLM_PROTOCOL_DECISIONS.md`](../../.github/LLM_PROTOCOL_DECISIONS.md), not here.
|
||||||
|
- Cross-cutting issues / TODOs (without architectural decision component) live in [`docs/XCUT/`](../XCUT/README.md).
|
||||||
Loading…
Reference in New Issue