From 70ae6f4f3c8af3dfebb66cd2f926fb1b669b3614 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:54:06 +0000 Subject: [PATCH] chore(sync): rebuild wff-code/wff-chat from .claude@fd99eb0 --- plugins/wff-chat/skills/git/workflows/pr.md | 9 +- .../SKILL.md | 21 +++ .../skills/shipping-and-launch/SKILL.md | 27 +++- plugins/wff-code/agents/code-reviewer.md | 1 - .../wff-code/agents/compliance-synthesis.md | 132 ++++++++++++++++-- .../agents/ossf-compliance-auditor.md | 4 +- plugins/wff-code/agents/owasp-agent.md | 42 +++++- plugins/wff-code/agents/owasp-api.md | 47 +++++++ plugins/wff-code/agents/owasp-citizen.md | 123 ++++++++++++++++ plugins/wff-code/agents/owasp-llm.md | 36 +++++ plugins/wff-code/agents/owasp-ml.md | 52 +++++++ plugins/wff-code/agents/owasp-web.md | 84 +++++++++++ plugins/wff-code/skills/git/workflows/pr.md | 9 +- .../context/handoff-quality-standards.md | 4 +- .../SKILL.md | 21 +++ .../skills/pr-review/workflows/pr-fix.md | 45 ++---- .../skills/pr-review/workflows/pr-review.md | 32 ++--- .../templates/roadmap-template.md | 1 - .../wff-code/skills/repo-compliance/SKILL.md | 59 +++++++- .../workflows/interactive-mode.md | 3 + .../skills/shipping-and-launch/SKILL.md | 27 +++- .../wff-code/skills/task-observer/SKILL.md | 52 +++---- 22 files changed, 703 insertions(+), 128 deletions(-) diff --git a/plugins/wff-chat/skills/git/workflows/pr.md b/plugins/wff-chat/skills/git/workflows/pr.md index 89eab48..1529600 100644 --- a/plugins/wff-chat/skills/git/workflows/pr.md +++ b/plugins/wff-chat/skills/git/workflows/pr.md @@ -79,9 +79,6 @@ Identify: - **Purpose**: Why these changes were made - **Impact**: Benefits, risks, breaking changes - **Testing**: What validation was done or needed -- **CHANGELOG**: If any commit uses type `feat`, `fix`, `perf`, or includes `!` (breaking - change), verify that `CHANGELOG.md` has been updated. If not, note it as a required - action before PR creation. - **Size**: Count total lines changed (`git diff $(git merge-base HEAD main)..HEAD --stat | tail -1`). If > 500 lines, consider whether the PR can be split. Recommend a split when changes span unrelated concerns (for example, a feature addition combined with a @@ -129,7 +126,6 @@ Omit this section for pure code changes with no deployment side-effects.] - [ ] Format passes (`uv run ruff format --check`) - [ ] Linting passes (`uv run ruff check`) - [ ] Type checking passes (`uv run basedpyright`) -- [ ] CHANGELOG.md updated (required for feat, fix, perf, or breaking changes) ## Notes @@ -200,7 +196,7 @@ time: 1. **Re-sync immediately before merge.** `gh pr merge --auto` plus update-branch is the efficient loop, but the PR must be up-to-date against the latest base at merge time. -2. **Apply skip/changelog labels BEFORE the synchronize push.** A label only takes effect if +2. **Apply skip labels BEFORE the synchronize push.** A label only takes effect if present before the push that triggers the workflows. Applying it after requires an empty commit to re-trigger; a bare label event does not re-run `pull_request` workflows. 3. **Dismiss stale bot reviews.** A `coderabbit`/Copilot review left in CHANGES_REQUESTED @@ -213,7 +209,7 @@ time: 6. **Diff the merged file vs base to verify semantic correctness.** A textual auto-merge can be semantically wrong. Conflict-resolution heuristic: for files already changed by merged PRs take the base branch's version (never revert merged work); keep the PR's version only - for files unique to it; union additive doc sections (e.g., CHANGELOG). + for files unique to it; union additive doc sections (e.g., a reference index or catalog). Verify the actual gate state (`gh pr view --json mergeStateStatus,statusCheckRollup`) rather than waiting on a never-reported required context. @@ -267,7 +263,6 @@ unblocks the enterprise customer pilot that requires SSO. Tracks #142. - [x] Format passes (`uv run ruff format --check`) - [x] Linting passes (`uv run ruff check`) - [x] Type checking passes (`uv run basedpyright`) -- [x] CHANGELOG.md updated ## Notes diff --git a/plugins/wff-chat/skills/observability-and-instrumentation/SKILL.md b/plugins/wff-chat/skills/observability-and-instrumentation/SKILL.md index a8acff0..e771f14 100644 --- a/plugins/wff-chat/skills/observability-and-instrumentation/SKILL.md +++ b/plugins/wff-chat/skills/observability-and-instrumentation/SKILL.md @@ -201,6 +201,27 @@ Rules for every alert you create: 4. Use two severities only: **page** (user-facing, act now) and **ticket** (degradation, act this week). A third tier becomes noise that trains people to ignore everything. +**Security alerting is a distinct axis from symptom alerting.** The symptom rules +above optimize for "users are hurt", which is the right default for reliability +and the wrong one for an attack in progress: credential stuffing, enumeration, and +scripted abuse degrade nothing a user feels until they succeed. Add rules for +authentication-failure and authorization-denial spikes alongside the symptom set. +Standards manifest `OPS-006` covers this and requires three things a reliability +alert usually leaves implicit: the rule is **committed** (not configured only in a +vendor console), it **names its destination channel**, and it carries a recorded +**test-fire timestamp**. An alert that pages nobody is not alerting. + +Related manifest checks in the `operations` domain +(`docs/standards-manifest.yaml`): `OPS-005` (security events emitted against a +documented taxonomy, and greppable in source, so a taxonomy cannot document +events the code never emits) and `OPS-004` (log secret redaction, proven by a +test rather than by the redaction helper's presence). + +This skill is the human-facing narrative for instrumenting one service. The +`OPS-*` checks are the durable half: they land in the compliance master log, get +delta caching, fleet escalation at the 3-repo threshold, and staleness detection, +none of which a skill provides. Use both. + ### 7. Verify the telemetry itself Instrumentation is code; it can be wrong. Before calling the work done, trigger the diff --git a/plugins/wff-chat/skills/shipping-and-launch/SKILL.md b/plugins/wff-chat/skills/shipping-and-launch/SKILL.md index 57d66b4..1886424 100644 --- a/plugins/wff-chat/skills/shipping-and-launch/SKILL.md +++ b/plugins/wff-chat/skills/shipping-and-launch/SKILL.md @@ -46,8 +46,13 @@ looks like. Every launch should be reversible, observable, and incremental. - [ ] Input validation on all user-facing endpoints - [ ] Authentication and authorization checks in place - [ ] Security headers configured (CSP, HSTS, etc.) -- [ ] Rate limiting on authentication endpoints +- [ ] Rate limiting on authentication endpoints, with the configured limit recorded (`OPS-011`) - [ ] CORS configured to specific origins (not wildcard) +- [ ] Application data-store role is not the table owner and does not hold BYPASSRLS (`OPS-002`) +- [ ] Row-level security, if claimed, passes a two-direction test (`OPS-003`) +- [ ] Logs redact secrets, proven by a test rather than by the filter's presence (`OPS-004`) +- [ ] Public write paths (signup, password reset, contact forms) carry an anti-automation control (`OPS-009`) +- [ ] Runtime secrets come from a secret manager, not a baked-in `.env` (`OPS-010`) ### Performance @@ -64,6 +69,8 @@ looks like. Every launch should be reversible, observable, and incremental. - [ ] On-call questions for this feature are answerable from telemetry - [ ] Logging and error reporting configured - [ ] Symptom-based alerts created and test-fired +- [ ] Security events emitted against a documented taxonomy: authn failure, authz denial, rate-limit trip, input-validation reject (`OPS-005`) +- [ ] Security alerts are committed as rules, name their destination channel, and carry a recorded test-fire timestamp (`OPS-006`) ### Accessibility @@ -76,18 +83,32 @@ looks like. Every launch should be reversible, observable, and incremental. ### Infrastructure -- [ ] Environment variables set in production +- [ ] Environment variables set in production, and attested in a dated runtime-config document (`OPS-001`) - [ ] Database migrations applied (or ready to apply) - [ ] DNS and SSL configured - [ ] CDN configured for static assets - [ ] Health check endpoint exists and responds +- [ ] Backups inventoried with schedule, retention, and destination (`OPS-007`) +- [ ] A restore drill has actually been performed and logged; a configured backup is not a tested one (`OPS-008`) +- [ ] Managed-service console settings are committed AND pushed by a workflow, not just committed (`OPS-012`) + +> **`OPS-*` references above point to the `operations` domain in +> `docs/standards-manifest.yaml`.** This checklist is the human-facing narrative +> for a single deploy; the manifest checks are the durable half that lands in the +> master log, gets delta caching, fleet escalation at the 3-repo threshold, and +> staleness detection. Use both. A checkbox ticked here leaves no record; an +> `OPS-*` finding does. +> +> The manifest checks are deliberately harder to satisfy than a checkbox: each +> names a durable artifact rather than a state of the world. "Backups are +> configured" ticks a box; `OPS-008` wants a dated restore-drill log entry. ### Documentation - [ ] README updated with any new setup requirements - [ ] API documentation current - [ ] ADRs written for any architectural decisions -- [ ] CHANGELOG updated (OpenSSF release gate) +- [ ] Release commits are Conventional so semantic-release generates the CHANGELOG at release (OpenSSF release gate; do not hand-edit CHANGELOG.md) - [ ] User-facing documentation updated (if applicable) ## Feature Flag Strategy diff --git a/plugins/wff-code/agents/code-reviewer.md b/plugins/wff-code/agents/code-reviewer.md index 9e3b122..2dcf8ef 100644 --- a/plugins/wff-code/agents/code-reviewer.md +++ b/plugins/wff-code/agents/code-reviewer.md @@ -66,7 +66,6 @@ forward from this repo's prior review checklist: - [ ] Public APIs are documented - [ ] Complex logic has comments - [ ] README updated if needed -- [ ] CHANGELOG entry added ### Security diff --git a/plugins/wff-code/agents/compliance-synthesis.md b/plugins/wff-code/agents/compliance-synthesis.md index f2d0395..d3e07f0 100644 --- a/plugins/wff-code/agents/compliance-synthesis.md +++ b/plugins/wff-code/agents/compliance-synthesis.md @@ -39,6 +39,76 @@ fleet-wide insights, writes a weekly synthesis report. distinct repos. Promote groups appearing in three or more distinct sessions across two or more distinct repos. + **Exclude degenerate patterns before grouping.** A `pattern` that is + empty, whitespace-only, or a bare YAML block-scalar marker (`>-`, `>`, + `|`, `|-`, `|+`, `>+`) is a parse artifact, not a description. + Grouping them would make a parse bug the headline insight of the + report. Exclude them from trending, and report the count once under + Data Quality with the affected `session_date` and `repo` values so the + pollution stays visible instead of being silently dropped. + + **The parser fallback is a parse artifact too.** When a description + is unusable, `scripts/compliance_rollup_reconcile.py` writes + `"(no description parsed) "` so the candidate + stays traceable to its origin. That string is neither empty nor a + block-scalar marker, so the filter above does not catch it, and every + fallback row for the same ID normalizes to the same text: exactly the + shape that ranks first under the promotion rule. + + Classify a row as a fallback only when its `pattern` equals + `"(no description parsed) "` followed by that same candidate's own + `proposed_manifest_id`, and use that one predicate for exclusion, for + the Data Quality count, and for the `#VERIFY` step below. A prefix + test is the wrong shape here: the producer accepts arbitrary strings, + so a real description that happens to open with those words would be + discarded as an artifact, and a report that silently drops real + candidates is a worse failure than the pollution this rule prevents. + Matching the ID as well as the prefix is what makes the row + self-identifying rather than merely prefix-shaped. + + Exclude fallback rows from trending, count them separately from the + marker rows, and list their affected `session_date` and `repo` values + under Data Quality. A rising fallback count is a producer-side parse + problem, which is a different finding from a recurring real pattern + and must not be reported as one. + + The two artifact classes need separate assumptions and separate + verification steps, because they are found by different predicates. A + single `#VERIFY` keyed on the marker set cannot see a fallback row at + all: lines above establish that the marker filter does not catch + `"(no description parsed) ..."`. Pointing the fallback claim at the + marker predicate would return zero every time and read as evidence + that there is no fallback problem, which is a verification step that + cannot fail, the exact defect this whole rule set exists to remove. + + `#ASSUME` (markers): the committed master log carries 20 block-scalar + marker rows from an earlier parser version, spread across four + sessions and four repos, the shape that ranks first under the + promotion rule above. Measured once, at authoring time. + `#VERIFY` (markers): recount before citing. Filter + `master-log.jsonl` for `unclassified_candidates[*].pattern` values in + the block-scalar marker set (`>-`, `>`, `|`, `|-`, `|+`, `>+`), plus + empty and whitespace-only values, and report the observed count and + the distinct `(session_date, repo)` pairs you actually found, not the + number above. If they disagree, report what you measured and note the + drift. + + `#ASSUME` (fallback): the fallback-row count is unmeasured. No number + is asserted here, because none was taken. + `#VERIFY` (fallback): count them with the exact-equality predicate + defined above, `pattern` equal to `"(no description parsed) "` plus + that same candidate's own `proposed_manifest_id`, never the marker + set and never a bare prefix test. Report the count and the distinct + `(session_date, repo)` pairs separately from the marker figures. + + `#ASSUME`: the producer-side guard is `is_degenerate_pattern` in + `scripts/compliance_rollup_reconcile.py`, and these rows predate it, + so no new ones should appear. + `#VERIFY`: confirm that function still exists and still covers the + marker set above; if any degenerate row carries a `session_date` + later than the guard's introduction, the guard has a hole and that + is itself a Data Quality finding, not a historical artifact. + **Stuck manifest candidates.** For each unique `proposed_manifest_id` seen anywhere in the window: check whether that ID exists in the standards manifest at @@ -51,11 +121,34 @@ fleet-wide insights, writes a weekly synthesis report. with `remediation_status: open` more than 14 days after the original proposal, surface as a follow-through gap. - **Coverage and override hotspots.** Coverage: for every catalog repo + **Coverage and override volume.** Coverage: for every catalog repo not archived, find the newest `session_date`. Flag any older than - 60 days. Overrides: count `totals.overrides_applied` per check ID - across the window. Flag check IDs overridden in four or more - distinct repos. + 60 days. + + Overrides: `totals.overrides_applied` is a scalar integer per session + entry, not a per-check map, and `findings_by_check` items carry only + `{id, severity, remediation_status}` with no override field. + + `#ASSUME`: no producer writes a check-ID-to-override association + anywhere, so the per-check grouping this insight originally specified + cannot be computed from the current schema. That was read off the + producers once, at authoring time; if one later emits the + association, the scalar-only rule below turns from honest into + suppressive and hides attribution that is now observable. + `#VERIFY`: before applying the rule, grep the master-log producers + (`scripts/compliance_rollup_reconcile.py` and the + `compliance-retrospective` agent's output schema) for an override + field on `findings_by_check` items. If one exists, report the real + per-check attribution and raise the stale rule as a Data Quality + finding against this file. + + Until a producer emits it, report + the scalar total per session and repo and label it explicitly as + "not attributable to specific check IDs (schema gap)". Do NOT invent + an attribution by inference. Raise the schema gap once under + Data Quality rather than silently emitting an empty insight, because + an insight that always returns nothing is indistinguishable from one + that found nothing to report. 4. For each insight with actionable items, spot-check 1-2 supporting per-repo files via `Read` to confirm the master log is not stale. @@ -119,11 +212,32 @@ status table (Resolved / Still open / Not re-audited), recommendation.> -## Override hotspots - - +## Override volume + + + +## Data quality + + ## Recommended actions for next sprint diff --git a/plugins/wff-code/agents/ossf-compliance-auditor.md b/plugins/wff-code/agents/ossf-compliance-auditor.md index 43fa2d2..3134ca3 100644 --- a/plugins/wff-code/agents/ossf-compliance-auditor.md +++ b/plugins/wff-code/agents/ossf-compliance-auditor.md @@ -808,8 +808,8 @@ Five criteria currently GAP. For each: the exact badge questionnaire text, what ### change_control_release_notes_vulns (MUST) **Criterion text:** "The project MUST identify each vulnerability in its change log." -**What satisfies it:** CHANGELOG entries for security fixes explicitly cite the CVE ID (e.g., `CVE-2024-12345`). -**Remediation:** Add to CLAUDE.md release standard: "CHANGELOG entries that fix a security vulnerability MUST include the CVE ID if one has been assigned. Format: `- fix(security): resolve CVE-2024-XXXXX -- [brief description]`." Apply retroactively to any existing vulnerability fix entries in CHANGELOG.md. +**What satisfies it:** The generated CHANGELOG entry for a security fix cites the CVE ID (e.g., `CVE-2024-12345`). Because python-semantic-release renders the changelog from Conventional Commits at release, the CVE ID must originate in the fixing commit message. +**Remediation:** Do NOT hand-edit `CHANGELOG.md`; the org retired per-PR changelog edits (see `ByronWilliamsCPA/.github` PR #288) because they conflicted under the merge queue. Instead, ensure the fixing commit names the CVE so PSR surfaces it at release, e.g. `fix(security): resolve CVE-2024-XXXXX -- [brief description]`. Add to the CLAUDE.md release standard: "Commits that fix a security vulnerability MUST include the CVE ID in the commit message if one has been assigned." Existing pre-PSR entries already in `CHANGELOG.md` may be left as-is. ### reporting_vulnerability_report_private (MUST) diff --git a/plugins/wff-code/agents/owasp-agent.md b/plugins/wff-code/agents/owasp-agent.md index 67e946e..401ff05 100644 --- a/plugins/wff-code/agents/owasp-agent.md +++ b/plugins/wff-code/agents/owasp-agent.md @@ -9,7 +9,7 @@ tools: ["Read", "Grep", "Glob"] You are a security specialist with deep expertise in the OWASP Top 10 for Agentic Applications (2026 edition). You review code and tests for -security risks specific to autonomous AI agent systems — agents that +security risks specific to autonomous AI agent systems: agents that plan, act, and make decisions across complex workflows. ## Your Categories @@ -38,6 +38,14 @@ plan, act, and make decisions across complex workflows. - Network access without allowlist - Database access with write permissions when read-only suffices +**AG02 Prompt Injection:** + +- Untrusted content (fetched web pages, tool results, retrieved documents) concatenated directly into the agent's system or instruction prompt with no delimiter or sanitization step +- No structural distinction in the prompt-construction path between trusted developer/system instructions and untrusted user/tool/retrieved content +- Agent instructions assembled via string formatting or concatenation from external inputs rather than structured, role-tagged messages +- Tool output re-injected into the agent's instruction context without a trust boundary marker separating it from the original task +- No content filtering or length cap on untrusted text before it reaches the LLM context window + **AG03 Insecure Tool Design:** - Tool functions accepting arbitrary string parameters from LLM output @@ -45,6 +53,22 @@ plan, act, and make decisions across complex workflows. - Tools that execute shell commands or SQL from agent-provided input - No rate limiting on tool invocations +**AG04 Insufficient Sandboxing:** + +- Agent-invoked code execution (eval, exec, subprocess, shell invocation) running with no container, VM, or restricted execution boundary +- File system tools with no chroot/jail-style restriction confining access to a designated workspace directory +- No CPU, memory, or execution-timeout limit on agent-triggered code execution +- Agent given direct interpreter or shell access rather than a mediated, allowlisted tool call +- Sandboxed execution results trusted back into the agent context with no re-validation of what actually ran + +**AG05 Broken Authentication:** + +- Sub-agent or delegated task spawned with the parent agent's full credential set or token rather than a scoped, delegation-specific credential +- No verification of caller identity before an agent acts on behalf of a user, an unauthenticated "acting as user X" field taken at face value +- Shared API keys or service credentials reused across multiple agent identities with no per-agent scoping +- No expiration or single-use constraint on credentials issued for a delegated agent action +- Agent-to-agent calls accepted with no signature or mutual authentication check before the receiving agent acts on the instruction + **AG06 Inadequate Guardrails:** - No confirmation step for destructive/irreversible actions @@ -59,6 +83,22 @@ plan, act, and make decisions across complex workflows. - Memory injectable via user-controlled conversation history - No expiration or rotation of agent memory/context +**AG08 Insufficient Logging:** + +- Agent actions (tool calls, decisions, state changes) recorded with no correlation ID linking the action back to its initiating request or user +- Logging statements on high-risk actions (file writes, external calls, financial or destructive operations) that omit agent identity, tool name, or input/output +- No structured log schema for agent decisions, only free-text prints that cannot be queried or aggregated +- Catch blocks that swallow a tool-call failure without logging the failure or the action that was attempted +- Retention and queryability of the audit trail are runtime properties invisible from source; covered by standards manifest OPS-005 (security event logging taxonomy), evaluated by the `operations-posture-auditor` agent + +**AG09 Insecure Multi-Agent Communication:** + +- Shared mutable memory or state store written by one agent and read as trusted, unvalidated input by another agent +- No message-origin verification between agents, so any agent can inject a message claiming to be from a trusted peer +- No circuit breaker or isolation boundary preventing one agent's failure or hallucination from propagating unchecked to downstream agents +- Inter-agent messages passed as raw strings with no schema validation before being acted on +- No rate limit or budget cap on inter-agent delegation chains, allowing unbounded fan-out + **AG10 Supply Chain:** - MCP servers loaded from unverified sources diff --git a/plugins/wff-code/agents/owasp-api.md b/plugins/wff-code/agents/owasp-api.md index f2583bb..12ac2cf 100644 --- a/plugins/wff-code/agents/owasp-api.md +++ b/plugins/wff-code/agents/owasp-api.md @@ -36,6 +36,14 @@ vulnerabilities in REST, GraphQL, gRPC, and WebSocket APIs. - Missing `get_object_or_404(user=request.user)` patterns - Direct database queries using user-supplied IDs without filtering +**API02 Broken Authentication:** + +- Token decode calls that skip signature verification (`jwt.decode(..., options={"verify_signature": False})`, `verify=False`) +- Hardcoded secret or signing keys in source (`SECRET_KEY = "..."`, `JWT_SECRET = "..."`) +- Password or token comparisons using `==` instead of a constant-time compare (`hmac.compare_digest`, `secrets.compare_digest`) +- Missing expiration handling on tokens (no `exp` claim check) or refresh tokens issued without rotation +- Login or password-reset endpoints with no failed-attempt counter or lockout logic + **API03 Broken Object Property Level Authorization:** - Pydantic models or serializers that expose all fields by default @@ -50,12 +58,51 @@ vulnerabilities in REST, GraphQL, gRPC, and WebSocket APIs. - Missing rate limiting middleware - No query complexity limits on GraphQL endpoints +**API05 BFLA (Broken Function Level Authorization):** + +- Admin or privileged routes (`/admin/*`, `/internal/*`) with no role-check decorator or dependency +- Handlers missing `@requires_role(...)`, `@login_required` combined with a role check, or `Depends(require_admin)`-style equivalents +- Authorization logic that checks authentication only (`if request.user`) without checking `is_staff` / `is_admin` / a role claim +- One HTTP method on a resource guarded while a sibling method on the same route is not (GET protected, POST/PUT/DELETE open) +- GraphQL mutations with no per-mutation authorization resolver or directive + +**API06 Unrestricted Access to Sensitive Business Flows:** NOT STATICALLY DETECTABLE + +- Whether a flow is "sensitive" (bulk purchase, ticket reservation, referral payout) is a business judgment, not a code shape; separating legitimate high-volume use from automated abuse depends on runtime behavioral signal (request velocity, device fingerprint) that a single-pass source read cannot see +- Covered by: standards manifest `OPS-*` checks (domain: operations), evaluated by the `operations-posture-auditor` agent. Specifically OPS-009 (anti-automation on public write paths) and OPS-011 (authentication endpoint rate limiting with the limit recorded) +- Statically detectable sub-signals only: public write endpoints (checkout, signup, referral-claim, reservation) with no CAPTCHA or anti-automation middleware reference nearby; absence of any per-user or per-IP throttle decorator on flows that create financial or scarce-resource records + **API07 SSRF:** - User-supplied URLs passed to `requests.get()` or `httpx.get()` - Webhook URLs not validated against allowlist - Image/file fetching from user-provided URLs without restriction +**API08 Security Misconfiguration:** + +- Debug flags left enabled outside test config (`DEBUG = True`, `app.debug = True`) +- CORS configured with `allow_origins=["*"]` combined with `allow_credentials=True` +- Missing or permissive security headers in middleware (`X-Content-Type-Options`, `Content-Security-Policy`, `Strict-Transport-Security` unset) +- Exception handlers that return stack traces or internal error detail to the client +- Default or example credentials carried over into real config (`.env.example` values reused verbatim) +- Framework debug or introspection endpoints reachable in production (`/graphql` introspection enabled, `/actuator`, `/__debug__`) + +**API09 Improper Inventory Management:** + +- Route registrations with no version prefix, or mixed versions (`/v1/`, `/v2/`, unversioned) coexisting with no documented deprecation path +- Endpoints present in the router but absent from the OpenAPI/Swagger spec (diff route registrations against `openapi.json` / `swagger.yaml`) +- Commented-out or `# deprecated`-tagged routes that are still registered and reachable +- Multiple environment or gateway configs (staging, internal, partner) referencing hosts not covered by the documented API inventory +- Debug or test-only routes gated by a flag that defaults to enabled + +**API10 Unsafe Consumption of APIs:** + +- Third-party responses parsed and used with no schema or type validation (no Pydantic model, no `response.raise_for_status()`) +- Outbound HTTP client calls with no timeout set (`requests.get(url)` without `timeout=`) +- Automatic redirect following on calls to external services (`allow_redirects=True` with no host allowlist) +- TLS verification disabled on outbound calls (`verify=False`) +- Third-party response data interpolated directly into HTML, shell commands, or SQL without sanitization + ## Mode: review-tests / generate Generate API security tests using the project's test client (FastAPI diff --git a/plugins/wff-code/agents/owasp-citizen.md b/plugins/wff-code/agents/owasp-citizen.md index ac3062c..202e7c3 100644 --- a/plugins/wff-code/agents/owasp-citizen.md +++ b/plugins/wff-code/agents/owasp-citizen.md @@ -46,6 +46,129 @@ Focus on patterns unique to AI-assisted development: - AI-hallucinated package names or API endpoints - Missing input validation in AI-generated route handlers +### Detection Patterns + +**CD01 Identity, Authentication and Authorization Misuse:** + +- Route handlers with no auth decorator where a sibling handler in the same + module has one (`@login_required`, `Depends(get_current_user)`) +- Authorization enforced in the UI layer only, with the underlying API route + reachable unauthenticated +- One service-account credential reused across environments, or an OAuth client + ID and secret shared between development and production config +- Ownership checks missing on record lookups: a query keyed on a request ID with + no `user=` or `tenant=` filter +- Hardcoded bypass flags (`if user.email.endswith("@internal")`, `SKIP_AUTH`) + +**CD02 Security Misconfiguration:** + +- `DEBUG = True`, `app.run(debug=True)`, or `FastAPI(debug=True)` reachable from + a production settings path +- Admin interfaces mounted at a default path with no auth or IP restriction + (Django admin, `flask_admin`, `/admin`) +- `ALLOWED_HOSTS = ["*"]` or `CORSMiddleware(allow_origins=["*"], + allow_credentials=True)` +- Default or seeded credentials left in config, fixtures, or migration files +- Framework error pages returned to clients with stack traces intact + +**CD03 Injection Handling Failures:** + +- f-string, `.format()`, or `%` interpolation building SQL instead of + parameterized queries +- `subprocess.run(shell=True)`, `os.system()`, `eval()`, or `exec()` on any + value derived from a request +- Template output marked trusted on user input (`|safe`, `mark_safe`, + `dangerouslySetInnerHTML`) +- NoSQL queries built by splatting a raw request dict into a filter +- Generated code that validates on the client and not again on the server + +**CD04 Data and Privacy Exposure:** + +- PII written to logs: whole request or response bodies logged, or direct field + logging (`logger.info(user.email)`) +- Serializers exposing every column (`fields = "__all__"`, `SELECT *` returned + straight to a response) +- Sensitive fields persisted to plaintext columns with no encryption at rest +- Connector or export configured to pull entire tables when a few columns are + used +- No deletion path for user data, so a retention policy cannot be honoured + +**CD05 Insecure Component and Dependency Management:** + +- An imported module with no corresponding entry in any dependency manifest, + the signature of a hallucinated package +- Package names one edit away from a popular library (typosquat adjacency) +- Dependencies declared with no version constraint and no committed lockfile +- `pip install` or `npm install` from a git ref, arbitrary URL, or non-default + index +- Dependencies added in the same commit as the code that uses them, with no + advisory check in between + +**CD06 Excessive Permissions and Oversharing:** + +- OAuth scopes broader than the code exercises (full-mailbox or full-drive scope + for a single-folder feature) +- Cloud IAM policies with wildcard actions or resources (`"Action": "*"`) +- Share links or bucket ACLs set to anyone-with-the-link or public-read +- Connectors configured with account-wide access where record-level would do +- Database grants to the application role beyond the statements it issues + +**CD07 Insufficient Logging and Monitoring:** NOT STATICALLY DETECTABLE + +- Source analysis can see whether a log call exists; it cannot see whether the + log is retained, queryable, or watched. Whether an anomaly produces an alert + that reaches a human is a runtime property of a log sink and a notification + channel, neither of which is in the repository. +- Covered by: standards manifest `OPS-*` checks (domain: operations), evaluated + by the `operations-posture-auditor` agent, which is granted Bash to reach + runtime state. Specifically `OPS-005` (security events emitted against a + documented taxonomy, each greppable in source), `OPS-006` (alert rules + committed, naming a destination channel, with a recorded test-fire timestamp), + and `OPS-004` (log secret redaction proven by a test). +- Statically detectable sub-signals only: authentication failure, authorization + denial, and input-validation rejection branches that emit no log call at all; + exception handlers that swallow without logging. + +**CD08 AI-Assisted Code Vulnerabilities:** + +- Deprecated idioms typical of stale training data: `datetime.utcnow()`, + `hashlib.md5` for passwords, `random` for tokens, Python 2 constructs +- Placeholder values left in place: `your-api-key-here`, `example.com`, + `changeme`, `TODO: add validation` +- Over-broad exception handling that swallows silently (`except Exception: + pass`), a common generation artifact +- Calls to API methods that do not exist on the imported library version, the + hallucinated-API signature +- Near-duplicate handlers with divergent validation, from repeated generation + rather than refactoring + +**CD09 Insecure Secrets Management:** + +- Literal API keys, tokens, connection strings, or passwords in source, config, + fixtures, or notebook outputs +- A populated `.env` committed, or copied into an image by a Dockerfile `COPY` +- Secrets exposed to the client bundle through a public-prefixed variable + (`NEXT_PUBLIC_`, `VITE_`, `REACT_APP_`) +- Credentials passed as Docker build args, which persist in image layers +- Supplement: `OPS-010` covers the runtime half, whether the deployed process + sources secrets from a secret manager rather than a baked-in file. Report + secret NAMES only, never values, and defang any secret-shaped evidence. + +**CD10 Inadequate Governance and Oversight:** NOT STATICALLY DETECTABLE + +- Whether a deployment was reviewed, who approved it, and what settings a + citizen developer changed in a vendor console are facts about a process and a + dashboard, not about a tree. Shadow IT is by definition the work that left no + trace in the repository, so its absence from source is not evidence. +- Covered by: standards manifest `OPS-*` checks (domain: operations), evaluated + by the `operations-posture-auditor` agent. Specifically `OPS-012` + (managed-service console settings committed AND pushed by a workflow, so a + config file no workflow applies is caught rather than credited) and `OPS-001` + (deployed runtime configuration attested in a dated document). +- Statically detectable sub-signals only: absent CODEOWNERS; no PR template or + required-review configuration; a committed vendor config file (for example + `supabase/config.toml`) that no workflow ever applies. + ## Mode: review-tests / generate Verify that AI-generated code has been security-reviewed and that tests diff --git a/plugins/wff-code/agents/owasp-llm.md b/plugins/wff-code/agents/owasp-llm.md index cb23748..96e30f5 100644 --- a/plugins/wff-code/agents/owasp-llm.md +++ b/plugins/wff-code/agents/owasp-llm.md @@ -45,6 +45,22 @@ applications, and generate missing security tests. - LLM output returned to user without PII scrubbing - Training data containing sensitive records +**LLM03 Supply Chain:** + +- Unpinned model references, `from_pretrained("model-name")` with no revision or commit hash, or a floating tag such as `:latest` +- `trust_remote_code=True` passed to `from_pretrained` or an equivalent loader call +- Dependency manifests or lock files with no hash pinning for model, plugin, or tool packages +- Third-party plugins or tool packages loaded without a checksum or signature verification step +- Model or dataset artifacts fetched from an unofficial mirror or raw URL instead of a pinned, signed registry source + +**LLM04 Data and Model Poisoning:** + +- Training or fine-tuning data loaded from an unverified remote source, a raw URL fetch with no checksum or provenance check +- No validation or sanitization step between data ingestion and the training or fine-tuning call +- User-submitted content fed directly into fine-tuning or RLHF pipelines without review or quarantine +- Missing dataset versioning or provenance metadata before a training run +- Model checkpoints loaded from mutable or world-writable storage with no integrity verification + **LLM05 Improper Output Handling:** - LLM output rendered as HTML without escaping @@ -65,6 +81,22 @@ applications, and generate missing security tests. - No instruction defense against extraction attempts - Prompt returned in error messages or debug output +**LLM08 Vector and Embedding Weaknesses:** + +- Vector store queries issued with no tenant, namespace, or user filter, enabling cross-tenant retrieval +- Embeddings generated from raw, unsanitized user or document input before insertion into the index +- No access control on vector store write/upsert operations, allowing arbitrary document injection into a shared index +- Missing similarity-score threshold on retrieval, letting low-relevance or adversarial chunks into the prompt context +- Retrieved chunks inserted into prompts with no check that the chunk's source is authorized for the requesting user + +**LLM09 Misinformation:** + +- RAG output surfaced to the user with no citation or grounding check against the retrieved source +- No confidence or groundedness scoring before an LLM response is returned +- Missing fact-check or verification step for output containing specific external claims +- Generated content presented as authoritative with no disclaimer or human-review gate for high-stakes output +- No cross-check between a generated summary and its source documents before display + **LLM10 Unbounded Consumption:** - No token limit on user input @@ -78,9 +110,13 @@ Check whether tests exist for each category. Key test patterns: - LLM01: Prompt injection payload matrix (>=10 diverse patterns) - LLM02: PII not present in LLM responses +- LLM03: Model and dependency references pinned, trust_remote_code disallowed by default +- LLM04: Training/fine-tuning data provenance validated, poisoned-sample injection test - LLM05: Output escaping when rendered in HTML/SQL/shell context - LLM06: Tool invocation restricted to authorized set - LLM07: System prompt not extractable via adversarial input +- LLM08: Cross-tenant retrieval isolation enforced, similarity threshold rejects low-relevance chunks +- LLM09: Groundedness/citation check on generated output, hallucination regression cases - LLM10: Token limits enforced, costs capped, timeouts configured ## Mode: generate diff --git a/plugins/wff-code/agents/owasp-ml.md b/plugins/wff-code/agents/owasp-ml.md index 61d7622..db95428 100644 --- a/plugins/wff-code/agents/owasp-ml.md +++ b/plugins/wff-code/agents/owasp-ml.md @@ -43,6 +43,30 @@ processing systems. - No statistical integrity checks on training datasets - Missing data provenance tracking +**ML03 Model Inversion:** + +- API responses expose full probability vectors or raw logits instead of a top-1 label +- No differential privacy noise or output perturbation applied before returning predictions +- Explanation/interpretability endpoints (SHAP, LIME, gradient) exposed without access control +- No rate limiting or query budget on endpoints that return fine-grained confidence scores + +**ML04 Membership Inference:** + +- Prediction confidence, loss, or logit values returned to the caller at full precision +- No output rounding, binning, or noise applied to per-class scores before response +- Model trained without a differential privacy budget (no DP-SGD, no epsilon/delta accounting) +- No monitoring for repeated near-duplicate queries probing the same candidate record + +**ML05 Model Theft / Extraction:** + +- No query-rate limit or budget on inference endpoints, enabling systematic extraction via + bulk querying; the rate-limiting control itself is runtime-only, covered by standards + manifest `OPS-011` (endpoint rate limiting with the limit recorded) and `OPS-009` + (anti-automation on public write paths), evaluated by the `operations-posture-auditor` agent +- Full model outputs (logits, embeddings, gradients) returned instead of minimized responses +- No authentication or API key requirement on prediction endpoints +- Model architecture, hyperparameters, or version exposed in API metadata or error messages + **ML06 AI Supply Chain:** - `pickle.load()` on model files from external sources @@ -56,6 +80,34 @@ processing systems. - No evaluation for inherited biases or backdoors - Missing comparison between base and fine-tuned behavior +**ML08 Model Skewing:** + +- Feedback or labeling endpoint accepts writes without authentication +- User-submitted labels or corrections merged into training data with no review or approval step +- No anomaly detection on feedback distribution before it feeds retraining +- Retraining pipeline triggers automatically from an unvalidated feedback stream + +**ML09 Output Integrity:** + +- Inference output returned without integrity or provenance metadata (no signature, hash, + or model-version tag on the response) +- No validation of prediction output before it is consumed downstream, e.g. a post-inference + cache writable by untrusted callers +- Model serving response path lacks transport authentication between the inference service + and its consumer +- No check that a returned prediction matches the model's own deterministic output, leaving + in-transit or in-cache result tampering undetected + +**ML10 Model Poisoning:** + +- Model artifacts loaded from an unpinned or unverified source, with no hash or signature + check before load +- `pickle.load()` / `torch.load()` used on an untrusted or externally-sourced checkpoint +- Training pipeline allows direct writes to the model parameter store with no code review + or approval gate +- No integrity verification comparing deployed model weights against a known-good baseline +- Missing gradient-clipping or anomaly detection on distributed/federated training updates + ## Mode: review-tests / generate Generate tests per Testing Standards S11.8.4 (data poisoning), S11.8.5 diff --git a/plugins/wff-code/agents/owasp-web.md b/plugins/wff-code/agents/owasp-web.md index 4b8fd86..d975458 100644 --- a/plugins/wff-code/agents/owasp-web.md +++ b/plugins/wff-code/agents/owasp-web.md @@ -47,6 +47,42 @@ For each source file in the target path: - CORS misconfiguration (wildcard origins) - Missing CSRF protection on state-changing endpoints +**A02 Security Misconfiguration:** + +- `DEBUG = True`, `app.run(debug=True)`, or `FastAPI(debug=True)` reachable + from a production settings path +- Default or placeholder `SECRET_KEY` / `JWT_SECRET` values left in settings + modules +- `ALLOWED_HOSTS = ["*"]`, or `CORSMiddleware(allow_origins=["*"], + allow_credentials=True)` +- No security headers middleware: no `Strict-Transport-Security`, + `X-Content-Type-Options`, or `Content-Security-Policy` set anywhere +- Session cookies configured without `secure`, `httponly`, or `samesite` + (CWE-1004) + +**A03 Software Supply Chain Failures:** + +- Dependencies declared with no version pin and no committed lockfile + (`uv.lock`, `poetry.lock`, `package-lock.json`) +- `pip install` from a git ref, an arbitrary URL, or an `--index-url` pointing + at a non-default registry +- GitHub Actions referenced by mutable tag (`uses: actions/checkout@v4`) + instead of a full commit SHA +- `curl ... | bash` or `wget ... && sh` in Dockerfiles, install scripts, or + CI steps +- Container base images pulled by `:latest` rather than by digest + +**A04 Cryptographic Failures:** + +- `hashlib.md5()`, `hashlib.sha1()`, or bare `hashlib.sha256()` used for + password storage instead of `bcrypt`, `argon2`, or `scrypt` +- `random.random()` / `random.choice()` or time-seeded values used for tokens, + nonces, or salts instead of `secrets` / `os.urandom` +- `verify=False` on `requests` / `httpx` calls, or `ssl.CERT_NONE` / + `check_hostname = False` +- ECB mode (`AES.MODE_ECB`), a hardcoded IV, or a key literal in source +- Sensitive fields persisted to plaintext columns with no encryption at rest + **A05 Injection:** - f-string or .format() in SQL queries (use parameterized queries) @@ -55,6 +91,22 @@ For each source file in the target path: - Unsanitized input in template rendering - Path construction with user input without sanitization +**A06 Insecure Design:** + +- Sequential or otherwise predictable integer primary keys exposed directly as + public resource identifiers in routes (`/users/`, `/orders/`) + instead of a UUID or opaque token, enabling enumeration +- Multi-step workflows (checkout, password reset, onboarding) with no + state-machine check confirming the prior step completed before the handler + for a later step runs +- Business-rule limits (quantity, price, discount, transfer amount) enforced + only in frontend JavaScript, with no matching validation in the backend + handler that processes the same request +- State-mutating endpoints (payment capture, balance transfer) with no + idempotency-key or nonce check, allowing the same request to be replayed +- Debug, admin, or feature-flagged routes registered on the production router + with no environment guard around their registration + **A07 Authentication Failures:** - Weak hashing (MD5, SHA1, SHA256 without salt for passwords) @@ -63,6 +115,38 @@ For each source file in the target path: - Session tokens with insufficient entropy - Missing MFA enforcement on admin routes +**A08 Software and Data Integrity Failures:** + +- `pickle.load()` / `pickle.loads()`, or `yaml.load()` without + `Loader=yaml.SafeLoader`, applied to data from an untrusted source (network + request, file upload, queue message) +- Auto-update, plugin-loading, or artifact-fetch code that retrieves and + executes remote content with no signature or checksum verification (no + `hmac.compare_digest`, no `hashlib` digest check before use) +- CI/CD workflow steps that consume a prior job's build artifact with no + SHA or checksum pin, or that download and execute a script over plain HTTP +- `marshal.loads()`, `jsonpickle.decode()`, or a custom `__reduce__` / + `__setstate__` implementation that deserializes attacker-influenced payloads +- JWT or signed-token verification disabled or weakened: `verify_signature=False`, + `options={"verify_signature": False}`, or `algorithms=["none"]` accepted + +**A09 Security Logging and Alerting Failures:** NOT STATICALLY DETECTABLE + +- Source analysis cannot observe a log stream, an alert rule, or an outbound + notification channel. The absence of a logging call is detectable; the absence + of alerting *on* that log is not, and neither is whether an alert reaches a human. +- Covered by: standards manifest `OPS-*` checks (domain: operations), evaluated by + the `operations-posture-auditor` agent, which is granted Bash to reach runtime + state. Specifically `OPS-005` (security events emitted against a documented + taxonomy), `OPS-006` (alert rules committed, naming a destination channel, with a + recorded test-fire timestamp), and `OPS-004` (log secret redaction proven by a test). +- Statically detectable sub-signals only: absence of any logging call in + authentication or authorization failure branches (`except` blocks around login, + token validation, or permission checks with no `logger.*` / `logging.*` call); + exception handlers that swallow the error with no logging (`except Exception: + pass`); log statements that interpolate raw credentials, tokens, or session + identifiers into the message (a redaction gap, not an alerting gap) + **A10 Mishandling of Exceptional Conditions (NEW in 2025):** - Bare `except:` or `except Exception:` that silently swallows errors diff --git a/plugins/wff-code/skills/git/workflows/pr.md b/plugins/wff-code/skills/git/workflows/pr.md index 89eab48..1529600 100644 --- a/plugins/wff-code/skills/git/workflows/pr.md +++ b/plugins/wff-code/skills/git/workflows/pr.md @@ -79,9 +79,6 @@ Identify: - **Purpose**: Why these changes were made - **Impact**: Benefits, risks, breaking changes - **Testing**: What validation was done or needed -- **CHANGELOG**: If any commit uses type `feat`, `fix`, `perf`, or includes `!` (breaking - change), verify that `CHANGELOG.md` has been updated. If not, note it as a required - action before PR creation. - **Size**: Count total lines changed (`git diff $(git merge-base HEAD main)..HEAD --stat | tail -1`). If > 500 lines, consider whether the PR can be split. Recommend a split when changes span unrelated concerns (for example, a feature addition combined with a @@ -129,7 +126,6 @@ Omit this section for pure code changes with no deployment side-effects.] - [ ] Format passes (`uv run ruff format --check`) - [ ] Linting passes (`uv run ruff check`) - [ ] Type checking passes (`uv run basedpyright`) -- [ ] CHANGELOG.md updated (required for feat, fix, perf, or breaking changes) ## Notes @@ -200,7 +196,7 @@ time: 1. **Re-sync immediately before merge.** `gh pr merge --auto` plus update-branch is the efficient loop, but the PR must be up-to-date against the latest base at merge time. -2. **Apply skip/changelog labels BEFORE the synchronize push.** A label only takes effect if +2. **Apply skip labels BEFORE the synchronize push.** A label only takes effect if present before the push that triggers the workflows. Applying it after requires an empty commit to re-trigger; a bare label event does not re-run `pull_request` workflows. 3. **Dismiss stale bot reviews.** A `coderabbit`/Copilot review left in CHANGES_REQUESTED @@ -213,7 +209,7 @@ time: 6. **Diff the merged file vs base to verify semantic correctness.** A textual auto-merge can be semantically wrong. Conflict-resolution heuristic: for files already changed by merged PRs take the base branch's version (never revert merged work); keep the PR's version only - for files unique to it; union additive doc sections (e.g., CHANGELOG). + for files unique to it; union additive doc sections (e.g., a reference index or catalog). Verify the actual gate state (`gh pr view --json mergeStateStatus,statusCheckRollup`) rather than waiting on a never-reported required context. @@ -267,7 +263,6 @@ unblocks the enterprise customer pilot that requires SSO. Tracks #142. - [x] Format passes (`uv run ruff format --check`) - [x] Linting passes (`uv run ruff check`) - [x] Type checking passes (`uv run basedpyright`) -- [x] CHANGELOG.md updated ## Notes diff --git a/plugins/wff-code/skills/handoff/context/handoff-quality-standards.md b/plugins/wff-code/skills/handoff/context/handoff-quality-standards.md index b776a68..cabf277 100644 --- a/plugins/wff-code/skills/handoff/context/handoff-quality-standards.md +++ b/plugins/wff-code/skills/handoff/context/handoff-quality-standards.md @@ -55,8 +55,8 @@ paragraph three sections away. secondary edits that must accompany the primary change: - **Standards manifest check addition:** also update `last_updated` in the - manifest header, add a `### Added` CHANGELOG entry, classify the commit per - `manifest-changes.md` (feat vs fix) + manifest header and classify the commit per `manifest-changes.md` (feat vs + fix); the changelog is generated from the commit at release, not hand-edited - **Pre-commit hook addition:** also verify `rev:` is pinned to a SHA, add to `additional_dependencies` if needed, run `pre-commit autoupdate` or pin manually diff --git a/plugins/wff-code/skills/observability-and-instrumentation/SKILL.md b/plugins/wff-code/skills/observability-and-instrumentation/SKILL.md index a8acff0..e771f14 100644 --- a/plugins/wff-code/skills/observability-and-instrumentation/SKILL.md +++ b/plugins/wff-code/skills/observability-and-instrumentation/SKILL.md @@ -201,6 +201,27 @@ Rules for every alert you create: 4. Use two severities only: **page** (user-facing, act now) and **ticket** (degradation, act this week). A third tier becomes noise that trains people to ignore everything. +**Security alerting is a distinct axis from symptom alerting.** The symptom rules +above optimize for "users are hurt", which is the right default for reliability +and the wrong one for an attack in progress: credential stuffing, enumeration, and +scripted abuse degrade nothing a user feels until they succeed. Add rules for +authentication-failure and authorization-denial spikes alongside the symptom set. +Standards manifest `OPS-006` covers this and requires three things a reliability +alert usually leaves implicit: the rule is **committed** (not configured only in a +vendor console), it **names its destination channel**, and it carries a recorded +**test-fire timestamp**. An alert that pages nobody is not alerting. + +Related manifest checks in the `operations` domain +(`docs/standards-manifest.yaml`): `OPS-005` (security events emitted against a +documented taxonomy, and greppable in source, so a taxonomy cannot document +events the code never emits) and `OPS-004` (log secret redaction, proven by a +test rather than by the redaction helper's presence). + +This skill is the human-facing narrative for instrumenting one service. The +`OPS-*` checks are the durable half: they land in the compliance master log, get +delta caching, fleet escalation at the 3-repo threshold, and staleness detection, +none of which a skill provides. Use both. + ### 7. Verify the telemetry itself Instrumentation is code; it can be wrong. Before calling the work done, trigger the diff --git a/plugins/wff-code/skills/pr-review/workflows/pr-fix.md b/plugins/wff-code/skills/pr-review/workflows/pr-fix.md index d85c08a..9356eaf 100644 --- a/plugins/wff-code/skills/pr-review/workflows/pr-fix.md +++ b/plugins/wff-code/skills/pr-review/workflows/pr-fix.md @@ -190,14 +190,12 @@ For each check with `conclusion` not `success` and not `neutral`, do the followi | basedpyright, type | Type-check | Fix annotations | | Bandit, Security, security-analysis | Security | Fix flagged patterns | | Dead Code, vulture | Dead code | Remove (confidence >= 90%) | -| Changelog | Changelog | Add entry from PR title | | Link, lychee | Links | Fix broken doc links | | REUSE, License | License | Add/fix headers | | Compatibility | Py version | Fix 3.10+ incompatibilities | | SBOM | SBOM | Fix dependency declarations | | SonarCloud | Quality gate | Defer to Step 1c | | qlty | Quality gate | Defer to Step 1c handling; enumerate locally if the qlty CLI is available (see Step 5b) | -| Changelog | Changelog (label-aware) | If the gate's pass condition is "CHANGELOG edited OR skip-label present" (`if: !contains(...labels..., 'skip-changelog')`), the remedy is commit-type-dependent: release-impacting commits (feat/fix/perf/breaking) need an entry; docs/chore/test/refactor need the skip label. See Step 4 Changelog enforcement for the label + re-trigger ordering. | | Reusable workflow startup_failure (0 jobs, no logs, "workflow file issue") | Workflow-load failure | Not a step failure; diagnose at file/reference level. Check `uses:@` reachability via `gh api repos///compare/...`; if `diverged` (orphaned by a squash-merge), re-pin to a SHA reachable from the reusable repo's default branch that contains the file and exposes the same `workflow_call` inputs. `contents?ref=` serves dangling commits, so existence checks mislead; use `compare`. Validate cheaply with `workflow_dispatch` on a throwaway branch (startup validation runs at load time, before job `if:`). When a failure appears after an edit, confirm causation by reverting the suspected change on the current base before committing to a fix direction. | | Failing reusable-workflow check (job renders as ` / `, caller uses `uses: org/repo/...@`) and the FIX is to the workflow body | Wrong-ref fix risk | Before authoring a fix, resolve the running definition. For a workflow consumed via `uses: ...@`, the running body is whatever that SHA resolves to; it is NOT necessarily the reusable repo's default branch. Read the caller's pinned ref and `gh api compare` it against main AND any open-PR branch heads to identify which definition actually runs and will become canonical. A fix landed on the wrong ref (e.g. main, when the caller pins a diverged in-flight rework branch) is cosmetic, will not clear the observed failure, and can collide with an open rework PR of the same file. Fix the ref that runs. | | GitGuardian | Secrets | Alert user only, never auto-fix | @@ -368,7 +366,7 @@ partition must drive pr-fix prioritisation and the user-facing plan, not just th tier. **Classify each fix as code-changing vs GitHub-metadata-only** (PR title/body edits, -thread resolution, label changes such as `skip-changelog`, comment replies). Record +thread resolution, label changes, comment replies). Record `HAS_CODE_FIXES = true` only if at least one fix touches a working-tree file. This gates Step 3: a fix set that mutates no files needs no worktree. @@ -489,8 +487,8 @@ pinned to a CI-supported version before any Step 5a gate or commit: **Edit precondition is path-specific (worktree vs main tree).** The Edit tool's "file has been read" precondition keys on the exact absolute path, not on content. A -file Read earlier from the main tree (`/repo/CHANGELOG.md`) does NOT satisfy an Edit on -the worktree copy (`/repo/.worktrees/fix-prN/CHANGELOG.md`); the Edit rejects with "File +file Read earlier from the main tree (`/repo/README.md`) does NOT satisfy an Edit on +the worktree copy (`/repo/.worktrees/fix-prN/README.md`); the Edit rejects with "File has not been read yet." Before editing ANY file inside the worktree, Read it from the worktree path first, ideally with offset/limit near the insertion point. Never edit a worktree file on the strength of having read its main-tree counterpart. @@ -562,33 +560,14 @@ and `uv.lock` and recreate the AG04 trust gap that Step 5a's tiers close). the test-fix category as "verification deferred to Step 5a" and proceed to the next category. -**Changelog enforcement (label-aware):** Check whether any commit on this branch (since -`git merge-base HEAD origin/{BASE_BRANCH}`) uses type `feat`, `fix`, `perf`, or -includes `!` (breaking change). - -- Release-impacting commits exist: generate an entry from the PR title, commit messages, - and changed files, and place it under `[Unreleased]` in CHANGELOG.md. -- No release-impacting commits (docs/chore/test/refactor only): a red required Changelog - check is cleared only by the remedy the workflow accepts, and merely noting "no entry - required" leaves the gate RED. Inspect the gate. When its pass condition is - "CHANGELOG edited OR a skip label present" (`if: !contains(...labels..., 'skip-changelog')`), - apply the repo's changelog-skip label (commonly `skip-changelog`, confirm via - `gh label list`) rather than adding a wrong entry or silently skipping: - - ```bash - gh pr edit "$PR_NUMBER" --repo "$OWNER/$REPO" --add-label skip-changelog - ``` - - **Re-trigger ordering matters.** The label only takes effect on a run whose triggering - event already carried it. If the workflow's `on:` block lacks `labeled` (commonly it is - only `[opened, synchronize, reopened]`), adding the label does nothing to the already-red - check, and a plain "re-run failed job" replays the original label-free payload and fails - again. Apply the label BEFORE the next push/rebase-push (or close+reopen) so the resulting - synchronize-event payload includes it. Inspect the `on:` types before assuming a label - change re-runs anything. - - If no skip-label bypass exists, note "CHANGELOG not required: no feat/fix/perf/breaking - changes on this branch" and surface the still-red gate to the user. +**Do NOT hand-edit `CHANGELOG.md` and do NOT apply changelog-skip labels.** The changelog +is generated at release time by python-semantic-release from Conventional Commits; there is +no per-PR changelog gate to satisfy. The org `Changelog Check` job is a deprecated no-op +that always passes (see `ByronWilliamsCPA/.github` PR #288), so a red required "Changelog" +check on any current repo indicates a stale pinned workflow ref, not a missing entry: +diagnose it as a workflow-load/ref issue, never by fabricating a `[Unreleased]` entry. The +release-impacting signal lives in the PR title and commit types, which the commit-type +validation below enforces. **Invalid commit-type fixes (non-interactive reword):** When a commit on the branch uses an invalid or non-allowed Conventional Commit type (a Critical CLAUDE.md violation), @@ -1599,7 +1578,7 @@ After pushing a narrow fix, check `gh api repos/{OWNER}/{REPO}/commits/$PUSH_SHA for each required context; if a required context is missing on the head, the PR is silently blocked. Re-trigger it by ensuring the final push touches that workflow's trigger paths (for example, bundle the fixes so the last commit also edits a path the required workflow watches, -such as a CHANGELOG entry). This is the same phantom/never-reported required-check failure +such as a file under that workflow's `paths:` filter). This is the same phantom/never-reported required-check failure mode pr-review documents, surfacing here via path-filtered re-triggers; a green prior run does not carry forward to a new head. diff --git a/plugins/wff-code/skills/pr-review/workflows/pr-review.md b/plugins/wff-code/skills/pr-review/workflows/pr-review.md index 6e15c46..6a52e4b 100644 --- a/plugins/wff-code/skills/pr-review/workflows/pr-review.md +++ b/plugins/wff-code/skills/pr-review/workflows/pr-review.md @@ -179,19 +179,8 @@ REQUIRED=$(gh api "repos/$OWNER/$REPO/branches/$BASE_BRANCH/protection/required_ - Failing check is NOT required and `MERGE_STATE` is `UNSTABLE`: emit `[Important]` with the annotation "non-required, does not block merge" plus the check's own remediation hint. Do not emit BUILD FAILING for a non-required check. Rigidly labelling a trivially - remediable non-required check (e.g., a Changelog gate clearable by a `skip-changelog` - label on a docs/test PR) as Critical overstates impact and pushes the user toward - unnecessary code changes. - -**Changelog-gate remedy is commit-type-dependent.** When the failing check is a Changelog -gate whose pass condition is "CHANGELOG.md edited OR a skip label present" (a job-level -`if: !contains(...labels..., 'skip-changelog')` guard), the recommended fix depends on the -branch's commit types, not on adding an entry reflexively. If the branch has no -`feat`/`fix`/`perf`/breaking commits (docs/chore/test/refactor only), the remedy is the -repo's changelog-skip label (commonly `skip-changelog`, confirm via `gh label list`), NOT -a fabricated CHANGELOG entry. Note the re-trigger ordering for pr-fix: the label only takes -effect on a run whose triggering event already carried it, so it must be applied BEFORE the -next push/reopen if the workflow's `on:` block lacks `labeled`. + remediable non-required check (e.g., a non-required Documentation Links or spell-check + gate) as Critical overstates impact and pushes the user toward unnecessary code changes. For each failing check in `CI_CHECKS`: @@ -488,7 +477,7 @@ only: - confirmation that the **dependency-security CI checks** (Dependency Review, Socket, Trivy, SonarCloud) are green. -Agents A (CLAUDE.md/CHANGELOG), C, D, J, and M still apply if not skipped by the +Agents A (CLAUDE.md), C, D, J, and M still apply if not skipped by the docs-only rule. **Trivial-change fast path (scale effort to the analyzable surface, not just line count).** @@ -824,12 +813,6 @@ or `.pre-commit-config.yaml` in the diff actually invokes it. A tool installed o Report: [Suggested] Dev dep "{tool}" declared but not wired to CI or pre-commit. -Also check: if the commit history contains any `feat:`, `fix:`, `perf:`, or `!` -(breaking) commit (run `gh api repos/{OWNER}/{REPO}/commits?sha={HEAD_SHA}&per_page=20` -and scan the commit messages), verify that `CHANGELOG.md` appears in CHANGED_FILES. -If it is absent, report: -`[Important] CLAUDE.md: CHANGELOG.md not updated for feat/fix/perf/breaking change` - Also check: if `.claude/settings.json` appears in CHANGED_FILES, verify all `Bash()` permission patterns use space syntax (e.g., `Bash(git *)`) not colon syntax (e.g., `Bash(git:*)`). Colon syntax is the MCP tool format and does not match shell commands; @@ -844,8 +827,9 @@ registration: a rename that reads as internally consistent in narrative document still be broken against the registry and fail at runtime. Report: [Critical] CLAUDE.md: renamed identifier "{new}" does not resolve in {.mcp.json|settings.json|live tool list}; the rename breaks every reference at runtime. -Also check commit types: if commit history is available (from the CHANGELOG check), -cross-check each commit type against the project's conventional-commits allowed-type +Also check commit types: fetch the commit history +(`gh api repos/{OWNER}/{REPO}/commits?sha={HEAD_SHA}&per_page=20` and scan the commit +messages), then cross-check each commit type against the project's conventional-commits allowed-type table (fetch `.claude/standards/conventional-commits.md` via `gh api repos/{OWNER}/{REPO}/contents/.claude/standards/conventional-commits.md`; if absent, use the default set: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert). Any commit type not in that table (e.g., `security:`, `ops:`, `claude:`) should be flagged: @@ -1397,7 +1381,7 @@ high scores and cluster at round numbers; these constraints correct both): 49 (Suggested) unless the finding evidences an actual untested code-behavior risk in the diff. A statically-verifiable no-op (e.g., a boolean input that is never read, default false) is not Critical. -- Doc-nit findings (missing CHANGELOG entry, doc count off-by-one, missing Bash +- Doc-nit findings (doc count off-by-one, missing Bash permissions allow rule, SKILL.md frontmatter gap, style/vocabulary inconsistency) are capped at 65 (Important) unless they break a build, lose data, or violate a hard CLAUDE.md rule. @@ -1675,7 +1659,7 @@ current diff. The PR was likely narrowed since that review. Verify the PR description still matches the current diff scope. ``` -Additionally, scan the branch commit subjects (already fetched for the CHANGELOG +Additionally, scan the branch commit subjects (already fetched for the commit-type check) for removal verbs: "remove", "drop", "revert", "strip ... from PR". When found alongside PR body claims of those artifacts, emit: diff --git a/plugins/wff-code/skills/project-planning/templates/roadmap-template.md b/plugins/wff-code/skills/project-planning/templates/roadmap-template.md index 9252b80..52ae7ac 100644 --- a/plugins/wff-code/skills/project-planning/templates/roadmap-template.md +++ b/plugins/wff-code/skills/project-planning/templates/roadmap-template.md @@ -130,7 +130,6 @@ Finalize testing, documentation, and release preparation. - ✅ All tests passing - ✅ No critical/high security issues - ✅ README covers all usage -- ✅ CHANGELOG updated ### Tasks | Task | Est. Hours | Status | diff --git a/plugins/wff-code/skills/repo-compliance/SKILL.md b/plugins/wff-code/skills/repo-compliance/SKILL.md index 89b7182..b0450e1 100644 --- a/plugins/wff-code/skills/repo-compliance/SKILL.md +++ b/plugins/wff-code/skills/repo-compliance/SKILL.md @@ -105,6 +105,7 @@ the precise gap it does not yet name explicitly (see escalation notes for this r | general | `general-compliance-auditor` | unclassified | | mkdocs | `mkdocs-auditor` | MKDOCS-* (skipped when mkdocs.yml absent) | | api | `openapi-compliance-agent` (via check-repo-compliance.py) | API-001..005 (applies_to: api_repos; skip when api.servesApi is false) | +| operations | `operations-posture-auditor` | OPS-* (applies_to: deployed_repos; skip when isDeployed is false, UNKNOWN when unset) | ### Pre-commit Domain: silent-skip wrapper defeats PC-* presence checks (obs 163) @@ -206,16 +207,58 @@ Robust patterns: Apply to any new bash check in this skill or in domain agent prompts that calls `gh api ... --jq` and tests for non-empty output. -### API Domain: applies_to Conditional +### applies_to Conditionals: tri-state, and never a silent skip -Before dispatching API-domain checks, read `api.servesApi` from the target -repo's catalog entry. The canonical catalog path is +This rule is universal. It governs every `applies_to` scope, not just the API +domain. The canonical catalog path is `${CLAUDE_HOME:-$HOME/.claude}/docs/reference/github-repos.json`; inside this repo it is also reachable at the relative path `docs/reference/github-repos.json`. Both refer to the same file (the `~/.claude` location is a symlink installed -by `setup.sh`). If `api.servesApi` is absent or `false`, skip all API-* -checks without raising FINDINGs; log `SKIP (api.servesApi: false)` in the -audit summary. API-* checks run only for repos where `api.servesApi: true`. +by `setup.sh`). + +| Scope | Catalog flag | Domain | Checks | +| --- | --- | --- | --- | +| `api_repos` | `api.servesApi` | api | API-001..005 | +| `docs_repos` | `publishesDocs` | mkdocs | MKDOCS-001..011, 013, 014 | +| `deployed_repos` | `isDeployed` | operations | OPS-* | + +Resolve each scope to one of three states. Absent is not false: + +| Flag value | Verdict | Behaviour | +| --- | --- | --- | +| `true` | APPLIES | Evaluate the domain normally. | +| `false` | SKIP | Skip the domain, log the SKIP line below, raise no FINDINGs. | +| absent, `null`, non-boolean, or repo absent from catalog | UNKNOWN | Skip the domain AND raise a FINDING saying the catalog needs populating. | + +**Coercing absent or null to false is the defect this rule exists to prevent.** +It silently reclassifies "nobody has declared" as "does not apply", which makes +a skipped domain indistinguishable from a passing one. The MkDocs domain shipped +in exactly that state: `publishesDocs` was absent on 44 of 45 catalog entries and +`false` on the 45th, so all 13 scoped MKDOCS-* checks evaluated to a silent skip +fleet-wide and the audit reported clean. A check that is named but cannot fail is +worse than a check that is absent. + +**Always print a line per scope, in every audit summary, including when the +count is zero.** A skip that prints nothing is the failure mode: + +```text +APPLICABILITY BY SCOPE +docs_repos (flag: publishesDocs, domain: mkdocs, 13 checks/repo) + APPLIES 0 repo(s) + SKIP (publishesDocs: false) 1 repo(s), 13 check evaluations skipped + UNKNOWN 1 repo(s), 13 check evaluations skipped -- catalog needs populating + [UNKNOWN] org/repo: publishesDocs absent from catalog entry; populate it +``` + +**Fleet-level reach assertion.** On a full sweep (not a single-repo run), any +scope where *zero* repos evaluate APPLIES is a dead domain: its checks exist, the +audit runs, and nothing is ever evaluated. Report it and fail the sweep. This is +the one place the MkDocs gap was detectable, because each individual repo's skip +looked locally correct. + +The deterministic path implements all of the above in +`scripts/check-repo-compliance.py` (`resolve_applicability`, +`render_scope_summary`, `assert_scopes_reachable`). The LLM path must match it. API-001 through API-003 are evaluated by `scripts/check-repo-compliance.py` via the GitHub Contents API. API-004 and API-005 read from the catalog directly @@ -320,6 +363,10 @@ Cached review data (domain-scoped): claude-docs-auditor: (no relevant cachedReview keys; verify on disk) ossf-compliance-auditor: scorecard, ossfBadge, codeql, secretScanning mkdocs-auditor: (no relevant cachedReview keys; verify on disk) + operations-posture-auditor: (no relevant cachedReview keys; the catalog + review schema describes repo and CI state, not deployed-system state. + This agent gathers its own evidence via Bash and the docs/operations/ + attestation files.) general-compliance-auditor: full cachedReview> ``` diff --git a/plugins/wff-code/skills/repo-compliance/workflows/interactive-mode.md b/plugins/wff-code/skills/repo-compliance/workflows/interactive-mode.md index a43f0a3..5446e1d 100644 --- a/plugins/wff-code/skills/repo-compliance/workflows/interactive-mode.md +++ b/plugins/wff-code/skills/repo-compliance/workflows/interactive-mode.md @@ -50,6 +50,7 @@ exactly: `branchProtection`, `codecov`, `codeql`, `foundations`, `ossfBadge`, - `claude-docs-auditor`: (no relevant cachedReview keys; verifies CLAUDE.md files on disk) - `ossf-compliance-auditor`: `scorecard`, `ossfBadge`, `codeql`, `secretScanning` - `mkdocs-auditor`: (no relevant cachedReview keys; verifies mkdocs.yml on disk) +- `operations-posture-auditor`: (no relevant cachedReview keys; the catalog `review` schema describes repo and CI state, not deployed-system state. This agent gathers its own evidence via Bash and the `docs/operations/` attestation files.) - `general-compliance-auditor`: full cachedReview (freeform review needs full context) Agents to dispatch simultaneously (skip any whose domain is in SKIP_DOMAINS): @@ -61,6 +62,7 @@ Agents to dispatch simultaneously (skip any whose domain is in SKIP_DOMAINS): - `ossf-compliance-auditor` (OSSF-* and SCORECARD:* checks) -- domain: `ossf` - `general-compliance-auditor` (all checks as negative filter, freeform review) -- never skipped - `mkdocs-auditor` in audit mode (MKDOCS-* checks; skipped automatically when no mkdocs.yml is present in the project root) -- domain: `mkdocs` +- `operations-posture-auditor` (OPS-* checks; applies_to `deployed_repos`. Skipped ONLY when the catalog `isDeployed` flag is explicitly `false`; absent, `null`, and non-boolean values still dispatch it so it emits the UNKNOWN finding) -- domain: `operations` > Note: REPO-* checks carry `domain: repo_settings` in the manifest but are produced by `repo-foundations-auditor`, which is dispatched under `domain: foundations`. A `SKIP_DOMAINS` entry of either `foundations` or `repo_settings` therefore skips the REPO-* checks, and any retrospective grouping should treat `repo_settings` findings as belonging to the foundations agent. @@ -127,6 +129,7 @@ Dispatch agents by domain in dependency order: 5. `claude-docs-auditor` (claude_docs: no dependencies) 6. `ossf-compliance-auditor` (ossf: no dependencies) 7. `mkdocs-auditor` in remediate mode (mkdocs: no dependencies; skipped automatically when no mkdocs.yml is present) +8. `operations-posture-auditor` in remediation mode (operations: no dependencies; skipped ONLY when `isDeployed` is explicitly `false`. Absent, `null`, and non-boolean values must still dispatch the agent so it emits the UNKNOWN finding; treating an undeclared flag as a skip reinstates the silent-skip defect this domain was built alongside, and all 45 catalog entries currently carry `isDeployed: null`). Its remediations are attestation scaffolds under `docs/operations/`, never live changes to a deployed system, a database role, or a vendor console. It must never rotate a credential, alter a database grant, or push a managed-service config; those are operator actions. It emits ACTION lines describing the scaffold it wrote and the operator step still required. Collect ACTION lines from each agent and present a summary of all changes made. diff --git a/plugins/wff-code/skills/shipping-and-launch/SKILL.md b/plugins/wff-code/skills/shipping-and-launch/SKILL.md index 57d66b4..1886424 100644 --- a/plugins/wff-code/skills/shipping-and-launch/SKILL.md +++ b/plugins/wff-code/skills/shipping-and-launch/SKILL.md @@ -46,8 +46,13 @@ looks like. Every launch should be reversible, observable, and incremental. - [ ] Input validation on all user-facing endpoints - [ ] Authentication and authorization checks in place - [ ] Security headers configured (CSP, HSTS, etc.) -- [ ] Rate limiting on authentication endpoints +- [ ] Rate limiting on authentication endpoints, with the configured limit recorded (`OPS-011`) - [ ] CORS configured to specific origins (not wildcard) +- [ ] Application data-store role is not the table owner and does not hold BYPASSRLS (`OPS-002`) +- [ ] Row-level security, if claimed, passes a two-direction test (`OPS-003`) +- [ ] Logs redact secrets, proven by a test rather than by the filter's presence (`OPS-004`) +- [ ] Public write paths (signup, password reset, contact forms) carry an anti-automation control (`OPS-009`) +- [ ] Runtime secrets come from a secret manager, not a baked-in `.env` (`OPS-010`) ### Performance @@ -64,6 +69,8 @@ looks like. Every launch should be reversible, observable, and incremental. - [ ] On-call questions for this feature are answerable from telemetry - [ ] Logging and error reporting configured - [ ] Symptom-based alerts created and test-fired +- [ ] Security events emitted against a documented taxonomy: authn failure, authz denial, rate-limit trip, input-validation reject (`OPS-005`) +- [ ] Security alerts are committed as rules, name their destination channel, and carry a recorded test-fire timestamp (`OPS-006`) ### Accessibility @@ -76,18 +83,32 @@ looks like. Every launch should be reversible, observable, and incremental. ### Infrastructure -- [ ] Environment variables set in production +- [ ] Environment variables set in production, and attested in a dated runtime-config document (`OPS-001`) - [ ] Database migrations applied (or ready to apply) - [ ] DNS and SSL configured - [ ] CDN configured for static assets - [ ] Health check endpoint exists and responds +- [ ] Backups inventoried with schedule, retention, and destination (`OPS-007`) +- [ ] A restore drill has actually been performed and logged; a configured backup is not a tested one (`OPS-008`) +- [ ] Managed-service console settings are committed AND pushed by a workflow, not just committed (`OPS-012`) + +> **`OPS-*` references above point to the `operations` domain in +> `docs/standards-manifest.yaml`.** This checklist is the human-facing narrative +> for a single deploy; the manifest checks are the durable half that lands in the +> master log, gets delta caching, fleet escalation at the 3-repo threshold, and +> staleness detection. Use both. A checkbox ticked here leaves no record; an +> `OPS-*` finding does. +> +> The manifest checks are deliberately harder to satisfy than a checkbox: each +> names a durable artifact rather than a state of the world. "Backups are +> configured" ticks a box; `OPS-008` wants a dated restore-drill log entry. ### Documentation - [ ] README updated with any new setup requirements - [ ] API documentation current - [ ] ADRs written for any architectural decisions -- [ ] CHANGELOG updated (OpenSSF release gate) +- [ ] Release commits are Conventional so semantic-release generates the CHANGELOG at release (OpenSSF release gate; do not hand-edit CHANGELOG.md) - [ ] User-facing documentation updated (if applicable) ## Feature Flag Strategy diff --git a/plugins/wff-code/skills/task-observer/SKILL.md b/plugins/wff-code/skills/task-observer/SKILL.md index 2eee89b..b999383 100644 --- a/plugins/wff-code/skills/task-observer/SKILL.md +++ b/plugins/wff-code/skills/task-observer/SKILL.md @@ -173,35 +173,29 @@ session. The user should not be interrupted by the logging process. write it to the log file within the same turn or the immediately following turn — do not accumulate observations in memory for batch-writing later.** The act of writing is the enforcement mechanism; mental notes are not observations. -Tie observation flushing to existing workflow checkpoints — e.g., when marking -a TodoWrite item as completed, check whether any unlogged observations have -accumulated and write them before proceeding. - -**Mandatory observation checkpoint after every 3rd TodoWrite completion:** After -marking the 3rd, 6th, 9th (etc.) TodoWrite item as completed in a session, -pause and explicitly ask: "Have any unlogged observations accumulated?" This is -a hard checkpoint, not a suggestion — the skill has demonstrated that softer -"check when completing items" guidance gets lost during cognitively demanding -analytical work. The count doesn't need to be precise; the rule is: roughly -every third completion, stop and flush. If nothing has accumulated, the pause -costs seconds. If observations have accumulated, this prevents the common -failure mode where the skill is loaded but no observations are written until -the user explicitly asks. - -**Subagent-controller sessions are the highest-risk case for this checkpoint.** -In a multi-task session where the controller dispatches implementer plus -reviewer subagents per task (a spec-compliance review and a code-quality review -each round), the cognitive load of coordinating those review loops reliably -crowds out observation logging. A six-task session ran roughly twelve subagent -dispatches with multiple correction moments and wrote zero observations until -the closing surfacing step. The "pause and ask" framing loses to this work -because it is a separate reminder that competes for attention. Bind the -checkpoint to an action that already happens: immediately before dispatching -the next implementer subagent (or marking the next task completed), WRITE any -accumulated observations to the log. The write is the checkpoint, not a mental -note to write later. Observation checkpoints that compete with cognitively -demanding work lose to that work; attaching the write to an unavoidable -existing step is what makes it stick. +Tie observation flushing to something that already happens, not to a separate +act of remembering. Writing at the moment of noticing is the primary +mechanism. The two backstops below are ordered by how little each depends on +the agent's attention. + +**Backstop 1, structural: the Stop hook.** At turn end, +`scripts/hooks/task-observer-flush-check.py` compares the observation count +against a baseline recorded at session start by +`scripts/hooks/task-observer-reminder.sh`. When a session used tools to +produce deliverables and logged nothing, the hook blocks turn end once and +asks for a flush. It depends on nothing the agent has to remember, which is +why it ranks first. An earlier version of this skill bound the checkpoint to +TodoWrite completions, so a session that never called TodoWrite had no trigger +at all; that gap is what the hook closes. + +**Backstop 2, behavioural: flush at batch boundaries.** Immediately before +dispatching the next subagent, marking a task item completed, or starting a +new unit of work, WRITE any accumulated observations. The write is the +checkpoint, not a mental note to write later. Subagent-controller sessions are +the highest-risk case: one six-task session ran roughly twelve subagent +dispatches with several correction moments and wrote zero observations until +the closing step. A checkpoint that competes with cognitively demanding work +loses to that work, so attach the write to a step that has to happen anyway. **Before assigning any observation number, run a mandatory pre-logging step:** Search the entire log file for all lines matching the pattern `### Observation \d+:`,