diff --git a/README.md b/README.md index e6a64cf9..7b728514 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ the one sending hook requests like the ones above. The **Guardian Agent** sits o other end of the wire and decides what happens next. A **hook** is a point in the Observed Agent's execution where it stops and asks before -acting. `toolCallRequest` is one of ACS's sixteen native lifecycle hooks. Others fire at +acting. `toolCallRequest` is one of ACS's nineteen native lifecycle hooks. Others fire at session start and end, before and after a turn, on knowledge retrieval, and around memory reads and writes. diff --git a/docs/concepts/skill.md b/docs/concepts/skill.md index 89bc81e1..98b180b7 100644 --- a/docs/concepts/skill.md +++ b/docs/concepts/skill.md @@ -30,4 +30,5 @@ Load is the runtime gate. It governs each activation in live context and carries - **Instrument**: the [`skillRegister`, `skillLoad`, and `skillUnload` hooks](../spec/instrument/hooks.md#skillregister). - **Inspect**: the `skill` [AgBOM component type](../spec/inspect/README.md). +- **Trace**: the [span names and OCSF classes](../spec/trace/events.md) the three hooks emit. - See also [Capability](./capability.md), [Provenance](./provenance.md), and [Trust basis](./trust.md). diff --git a/docs/spec/trace/events.md b/docs/spec/trace/events.md index 821b3b1c..0894b20b 100644 --- a/docs/spec/trace/events.md +++ b/docs/spec/trace/events.md @@ -34,6 +34,9 @@ The full mapping lives in [`trace/otel-mapping.json`](https://genai-security-pro | `steps/preCompact` | `acs.compact` | `acs.compact.entry_count`, `acs.compact.triggered_by` | | `steps/postCompact` | `acs.compact.complete` | `acs.compact.entry_count`; `acs.compact.lineage_depth_after` (optional) | | `steps/subagentStart`, `steps/subagentStop` | `acs.subagent`, `acs.subagent.end` | `acs.subagent.session_id`, `acs.subagent.parent_session_id`, `acs.subagent.intent_derivation` / `acs.subagent.outcome`, `acs.subagent.final_chain_hash` | +| `steps/skillRegister` | `acs.skill.register` | `acs.skill.id`, `acs.skill.digest.algorithm`, `acs.skill.digest.value`, `acs.skill.declared_capabilities` | +| `steps/skillLoad` | `acs.skill.load` | `acs.skill.id`, `acs.skill.digest.algorithm`, `acs.skill.digest.value`, `acs.skill.load_trigger`, `acs.skill.load_path` (skill ids, root first); `acs.skill.digest_verified`, `acs.skill.registration_ref` (optional) | +| `steps/skillUnload` | `acs.skill.unload` | `acs.skill.id`, `acs.skill.reason`; `acs.skill.replaced_by` (optional) | | Decision (allow/deny/modify/ask/defer) | `acs.decision` (span event) | `acs.decision`, `acs.evaluator`, `acs.reasoning` (when present), `acs.confidence` (when present) | | `agbom/snapshot` | `acs.agbom` | `acs.agbom.format` (`canonical`/`cyclonedx`/`spdx`/`swid`), `acs.agbom.component_count` | | `agbom/changed` | `acs.agbom` | `acs.agbom.format` (`canonical`/`cyclonedx`/`spdx`/`swid`), `acs.agbom.change_reason` | @@ -51,12 +54,14 @@ The full mapping lives in [`trace/ocsf-mapping.json`](https://genai-security-pro | `steps/sessionStart` | Authentication | 3002 | | `steps/agentTrigger`, `steps/userMessage`, `steps/agentResponse` | Application Activity | 6002 | | `steps/toolCallRequest`, `steps/toolCallResult` | Process Activity | 1007 | +| `steps/skillLoad`, `steps/skillUnload` | Module Activity | 1005 (`activity_id` 1 load, 2 unload) | | `steps/knowledgeRetrieval`, `steps/memoryStore`, `steps/memoryContextRetrieval` | Datastore Activity | 6005 | | `steps/preCompact`, `steps/postCompact` | Datastore Activity | 6005 (compaction subtype) | | `steps/subagentStart`, `steps/subagentStop` | Authentication | 3002 (logon/logoff for the subagent's session) | | `steps/turnStart`, `steps/turnEnd` | Application Activity | 6002 | | Decision (deny/modify/ask/defer) | Detection Finding | 2004 | | `agbom/snapshot`, `agbom/changed` | Inventory Info | 5001 | +| `steps/skillRegister` | Device Inventory Info | 5001 (skill enters the component set) | | `steps/sessionEnd` | Authentication | 3002 (logoff) | OCSF `severity_id` for decision events is set from the disposition: diff --git a/docs/spec/trace/extend_ocsf.md b/docs/spec/trace/extend_ocsf.md index b3921599..34464fc6 100644 --- a/docs/spec/trace/extend_ocsf.md +++ b/docs/spec/trace/extend_ocsf.md @@ -9,11 +9,13 @@ ACS events map to OCSF 1.5+ event classes. The normative class assignments and t | ACS step | OCSF class | Class UID | |---|---|---| | `steps/sessionStart`, `steps/sessionEnd`, `steps/subagentStart`, `steps/subagentStop` | Authentication | 3002 | -| `steps/userMessage`, `steps/agentResponse`, `steps/turnStart`, `steps/turnEnd` | Application Activity | 6002 | +| `steps/agentTrigger`, `steps/userMessage`, `steps/agentResponse`, `steps/turnStart`, `steps/turnEnd` | Application Activity | 6002 | | `steps/toolCallRequest`, `steps/toolCallResult` | Process Activity | 1007 | +| `steps/skillLoad`, `steps/skillUnload` | Module Activity | 1005 (`activity_id` 1 load, 2 unload) | | `steps/knowledgeRetrieval`, `steps/memoryStore`, `steps/memoryContextRetrieval`, `steps/preCompact`, `steps/postCompact` | Datastore Activity | 6005 | | Decision (deny/modify/ask/defer) | Detection Finding | 2004 | | `agbom/snapshot`, `agbom/changed` | Inventory Info | 5001 | +| `steps/skillRegister` | Device Inventory Info | 5001 | ## Severity mapping diff --git a/specification/v0.1.0/trace/ocsf-mapping.json b/specification/v0.1.0/trace/ocsf-mapping.json index 3856c8b5..47e3957e 100644 --- a/specification/v0.1.0/trace/ocsf-mapping.json +++ b/specification/v0.1.0/trace/ocsf-mapping.json @@ -34,6 +34,9 @@ "steps/postCompact": { "class_uid": 6005, "class_name": "Datastore Activity", "activity_id": 99 }, "steps/subagentStart": { "class_uid": 3002, "class_name": "Authentication", "activity_id": 1, "_note": "Subagent spawn opens a new session — modeled as Authentication/Logon for OCSF symmetry with sessionStart." }, "steps/subagentStop": { "class_uid": 3002, "class_name": "Authentication", "activity_id": 2 }, + "steps/skillRegister": { "class_uid": 5001, "class_name": "Device Inventory Info", "_note": "Registration admits a skill to the component set and persists its definition ref and digest to the AgBOM — modeled as Inventory Info, the class agbom/* already uses." }, + "steps/skillLoad": { "class_uid": 1005, "class_name": "Module Activity", "activity_id": 1, "_note": "OCSF defines Module Activity as a process loading or unloading a module, and its required module object is the skill itself — not Process Activity, which needs a launched or terminated process that a skill load does not have; keeping loads off 1007 lets a reader tell a skill loading from the tool calls its actions raise." }, + "steps/skillUnload": { "class_uid": 1005, "class_name": "Module Activity", "activity_id": 2 }, "agbom/snapshot": { "class_uid": 5001, "class_name": "Inventory Info" }, "agbom/changed": { "class_uid": 5001, "class_name": "Inventory Info" } } diff --git a/specification/v0.1.0/trace/otel-mapping.json b/specification/v0.1.0/trace/otel-mapping.json index d84846cb..d8cbfded 100644 --- a/specification/v0.1.0/trace/otel-mapping.json +++ b/specification/v0.1.0/trace/otel-mapping.json @@ -89,6 +89,20 @@ "span_name": "acs.subagent.end", "required_attributes": ["acs.subagent.session_id", "acs.subagent.outcome", "acs.subagent.final_chain_hash"] }, + "steps/skillRegister": { + "span_name": "acs.skill.register", + "required_attributes": ["acs.skill.id", "acs.skill.digest.algorithm", "acs.skill.digest.value", "acs.skill.declared_capabilities"] + }, + "steps/skillLoad": { + "span_name": "acs.skill.load", + "required_attributes": ["acs.skill.id", "acs.skill.digest.algorithm", "acs.skill.digest.value", "acs.skill.load_trigger", "acs.skill.load_path"], + "optional_attributes": ["acs.skill.digest_verified", "acs.skill.registration_ref"] + }, + "steps/skillUnload": { + "span_name": "acs.skill.unload", + "required_attributes": ["acs.skill.id", "acs.skill.reason"], + "optional_attributes": ["acs.skill.replaced_by"] + }, "agbom/snapshot": { "span_name": "acs.agbom", "required_attributes": ["acs.agbom.format", "acs.agbom.component_count"] diff --git a/tests/test_hook_taxonomy.py b/tests/test_hook_taxonomy.py index d21ace03..b40b674c 100644 --- a/tests/test_hook_taxonomy.py +++ b/tests/test_hook_taxonomy.py @@ -11,16 +11,66 @@ A reader treating the specification page as canonical never learned that skills are governable, and the omission propagated into implementations. The build stayed green throughout, so only a check that reads the schemas catches it. + +The same drift reached the Trace pillar. The OpenTelemetry and OCSF mapping +tables under `specification/v0.1.0/trace/`, the two docs pages that render +them, and the overview on the ACS page each restate the set, and none of them +gained the skill hooks. A deployment emitting OTel or OCSF from the mapping +files had no span name and no event class for a skill event, so the one hook +that vets a whole skill left no trace. The guards below therefore reach every +surface that restates the set: a file naming eight or more hooks as +`steps/` must name all of them, the mapping tables must key exactly the +schema set, and every hook count stated in the docs must match the schemas. + +Two kinds of page trip those rules on purpose: one that names a hook proposed +for a later version, and one that lists a deliberate subset of eight or more +hooks. Each failure message names its escape hatch, and the two allowlists +below record every exemption with a reason, so a deliberate subset stays +distinguishable from a stale copy in review. """ import json import re from pathlib import Path +import pytest + ROOT = Path(__file__).resolve().parent.parent HOOK_SCHEMAS = ROOT / "specification" / "v0.1.0" / "hooks" SPECIFICATION = ROOT / "docs" / "spec" / "instrument" / "specification.md" HOOKS_PAGE = ROOT / "docs" / "spec" / "instrument" / "hooks.md" +DOCS = ROOT / "docs" +TRACE_MAPPINGS = ROOT / "specification" / "v0.1.0" / "trace" +OTEL_MAPPING = TRACE_MAPPINGS / "otel-mapping.json" +OCSF_MAPPING = TRACE_MAPPINGS / "ocsf-mapping.json" + +# A file that names this many distinct hooks as `steps/` is restating +# the set rather than quoting an example. The Trace surfaces name fifteen or +# more; no example snippet in the docs names more than four. +RESTATING_THRESHOLD = 8 + +# Files that deliberately name a subset of RESTATING_THRESHOLD or more hooks as +# `steps/`, such as a table of the hooks ACS-Core gates. Path relative to +# the repo root -> why the subset is intended. Empty by default: an entry lands +# in the same PR as the page that needs it, where a reviewer can judge it. +SUBSET_SURFACES = {} + +# `steps/` references that deliberately name no schema, such as a hook +# proposed for a later version. Path -> {name: reason}. Keyed by file so one +# page's exemption does not hide the same name on another page. +PROPOSED_HOOKS = {} + +# Every place a new hook has to reach. Only some of them are checked below, so +# the omission messages print the whole list rather than only what failed. +NEW_HOOK_CHECKLIST = """ +A new hook reaches all of these in the same PR: + docs/spec/instrument/specification.md section 5 taxonomy row + docs/spec/instrument/hooks.md overview row, `## ` section, count + docs/spec/trace/events.md OTel and OCSF tables + docs/spec/trace/extend_ocsf.md OCSF class table + specification/v0.1.0/trace/*.json step_to_span and step_to_class keys + docs/spec/conformance.md, docs/acs.md ACS-Core or additional-hooks list + README.md native hook count""" # `steps/skillRegister payload` -> "skillRegister". The profile variants title # themselves `... payload (ACS-Provenance profile)` and are the same hook, so @@ -34,6 +84,31 @@ OVERVIEW_ROW = re.compile(r"^\|\s*\[?`([A-Za-z][A-Za-z0-9]*)`") # `ACS v0.1.0 defines 19 native `steps/*` hooks` -> "19" STATED_COUNT = re.compile(r"defines (\d+) native `steps/\*` hooks") +# `| `steps/skillRegister` | `acs.skill.register` |` -> "skillRegister". +# `steps/*` carries no name, so the wildcard on the Hooks page does not match. +# A glob (`steps/toolCall*`) names no single hook, and a path or anchor +# (`/docs/steps/x`, `#steps/x`) is not a method, so the lookarounds skip both. +STEPS_REFERENCE = re.compile(r"(? "16"; `sixteen native lifecycle hooks` -> +# "sixteen"; `19 native `steps/*` hooks` -> "19". "Native" is required, so the +# skill proposal's historical "Sixteen `steps/*` hooks" does not match. +STATED_COUNT_ANYWHERE = re.compile( + r"\b(\d+|" + "|".join(NUMBER_WORDS) + r") native (?:lifecycle |`steps/\*` )?hooks", + re.IGNORECASE, +) +# Where a native hook count may be stated. The proposals under +# `specification/proposals/` record the set as it stood and are left out. +COUNT_SURFACES = sorted(DOCS.rglob("*.md")) + [ROOT / "README.md"] def native_hook_methods(): @@ -47,10 +122,42 @@ def native_hook_methods(): return methods +def restating_surfaces(): + """Every file that may restate the hook set: docs pages and Trace mappings.""" + return sorted(DOCS.rglob("*.md")) + sorted(TRACE_MAPPINGS.glob("*.json")) + + +def steps_references(path): + """Distinct hook names a file references as `steps/`.""" + return set(STEPS_REFERENCE.findall(path.read_text(encoding="utf-8"))) + + +def mapping_keys(path, table): + """The `steps/*` keys of a Trace mapping's default table, prefix stripped. + + The tables also key `agbom/snapshot` and `agbom/changed`. Those are + Inspect-pillar methods that share the request envelope, not hooks, so they + sit outside the set the hook schemas define and are left out here. + """ + mapping = json.loads(path.read_text(encoding="utf-8")) + keys = mapping["properties"][table]["default"] + return {key[len("steps/") :] for key in keys if key.startswith("steps/")} + + +def relative(path): + return path.relative_to(ROOT).as_posix() + + def section(path, heading): """Lines under a `## ` heading, stopping at the next one.""" lines = path.read_text(encoding="utf-8").splitlines() - start = next(i for i, line in enumerate(lines) if line.startswith(heading)) + # A bare next() raises StopIteration, which pytest prints with no hint of + # which heading moved. + start = next((i for i, line in enumerate(lines) if line.startswith(heading)), None) + assert start is not None, ( + f"{relative(path)} has no heading starting {heading!r}. If it was renamed, " + "update the heading this test reads." + ) for offset, line in enumerate(lines[start + 1 :], start=start + 1): if line.startswith("## "): return lines[start + 1 : offset] @@ -70,9 +177,12 @@ def test_specification_taxonomy_lists_every_native_hook(): assert not missing, ( "Specification section 5 omits hooks the schemas define: " + ", ".join(missing) + + NEW_HOOK_CHECKLIST ) assert not unknown, ( - "Specification section 5 lists hooks with no schema: " + ", ".join(unknown) + "Specification section 5 lists hooks with no schema: " + + ", ".join(unknown) + + ". Add the schema under specification/v0.1.0/hooks/ or drop the row." ) @@ -95,10 +205,14 @@ def test_hooks_page_overview_lists_every_native_hook(): missing = sorted(expected - listed) unknown = sorted(listed - expected) assert not missing, ( - "Hooks page overview omits hooks the schemas define: " + ", ".join(missing) + "Hooks page overview omits hooks the schemas define: " + + ", ".join(missing) + + NEW_HOOK_CHECKLIST ) assert not unknown, ( - "Hooks page overview lists hooks with no schema: " + ", ".join(unknown) + "Hooks page overview lists hooks with no schema: " + + ", ".join(unknown) + + ". Add the schema under specification/v0.1.0/hooks/ or drop the row." ) @@ -107,5 +221,107 @@ def test_hooks_page_states_the_real_hook_count(): assert match, "Hooks page no longer states a native `steps/*` hook count" expected = len(native_hook_methods()) assert int(match.group(1)) == expected, ( - f"Hooks page claims {match.group(1)} native hooks; the schemas define {expected}" + f"Hooks page claims {match.group(1)} native hooks; the schemas define " + f"{expected}. Update the count on line 3 and in the diagram." + ) + + +def test_every_steps_reference_names_a_schema_hook(): + expected = native_hook_methods() + unknown = {} + for path in restating_surfaces(): + proposed = PROPOSED_HOOKS.get(relative(path), {}) + stray = sorted(steps_references(path) - expected - proposed.keys()) + if stray: + unknown[relative(path)] = stray + assert not unknown, ( + "Files reference `steps/` methods no schema defines:\n" + + "\n".join(f" {file}: {', '.join(hooks)}" for file, hooks in unknown.items()) + + "\nFix the typo, add the schema, or, for a hook proposed for a later " + "version, add it to PROPOSED_HOOKS in this file with a reason." + ) + + +def test_restating_surfaces_list_every_native_hook(): + expected = native_hook_methods() + missing = {} + for path in restating_surfaces(): + if relative(path) in SUBSET_SURFACES: + continue + listed = steps_references(path) + if len(listed) >= RESTATING_THRESHOLD and expected - listed: + missing[relative(path)] = sorted(expected - listed) + assert not missing, ( + f"Files naming {RESTATING_THRESHOLD} or more hooks as `steps/` are " + "treated as restating the set, and these omit hooks the schemas define:\n" + + "\n".join(f" {file}: {', '.join(hooks)}" for file, hooks in missing.items()) + + "\nAdd the missing hooks. If the file lists a subset on purpose, add it " + "to SUBSET_SURFACES in this file with a reason, or write the subset " + "without the `steps/` prefix." + + NEW_HOOK_CHECKLIST + ) + + +def test_allowlist_entries_carry_a_reason_and_are_still_needed(): + expected = native_hook_methods() + stale = [] + for file, reason in SUBSET_SURFACES.items(): + if not (ROOT / file).is_file() or not reason.strip(): + stale.append(f"SUBSET_SURFACES {file}: missing file or empty reason") + for file, names in PROPOSED_HOOKS.items(): + path = ROOT / file + for name, reason in names.items(): + if not path.is_file() or not reason.strip(): + stale.append(f"PROPOSED_HOOKS {file} {name}: missing file or empty reason") + elif name in expected or name not in steps_references(path): + # Once the hook ships, or the page drops it, the exemption only + # hides the next mistake. + stale.append(f"PROPOSED_HOOKS {file} {name}: no longer needed") + assert not stale, "Remove or fix these allowlist entries:\n " + "\n ".join(stale) + + +@pytest.mark.parametrize( + "path, table", + [(OTEL_MAPPING, "step_to_span"), (OCSF_MAPPING, "step_to_class")], + ids=["otel", "ocsf"], +) +def test_trace_mapping_keys_exactly_the_native_hooks(path, table): + keyed = mapping_keys(path, table) + expected = native_hook_methods() + + missing = sorted(expected - keyed) + unknown = sorted(keyed - expected) + assert not missing, ( + f"{path.name} {table} omits hooks the schemas define: " + + ", ".join(missing) + + NEW_HOOK_CHECKLIST + ) + assert not unknown, ( + f"{path.name} {table} keys hooks with no schema: " + + ", ".join(unknown) + + ". Add the schema under specification/v0.1.0/hooks/ or drop the key." + ) + + +def test_every_docs_page_states_the_real_hook_count(): + expected = len(native_hook_methods()) + stated = 0 + wrong = {} + for path in COUNT_SURFACES: + for match in STATED_COUNT_ANYWHERE.finditer(path.read_text(encoding="utf-8")): + word = match.group(1).lower() + count = NUMBER_WORDS.get(word) or int(word) + # "These 2 native hooks" counts a subset, never the whole set, which + # has always held more than RESTATING_THRESHOLD hooks. + if count < RESTATING_THRESHOLD: + continue + stated += 1 + if count != expected: + wrong.setdefault(relative(path), []).append(match.group(1)) + assert stated, "No docs page states a native hook count; the prose has moved" + assert not wrong, ( + f"Pages claim a native hook count other than the {expected} the schemas define:\n" + + "\n".join(f" {file}: {', '.join(counts)}" for file, counts in wrong.items()) + + f"\nUpdate the count. To count a subset, write it as \"9 of the {expected} " + "native hooks\" so the total stays checked." )