Skip to content

fix(trace): align OCSF provenance to unmapped.acs.provenance and add acs.provenance.ids - #335

Open
drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:spec/trace-provenance-join-260-262
Open

drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:spec/trace-provenance-join-260-262

Conversation

@drk-ram

@drk-ram drk-ram commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Aligns specification/v0.1.0/trace/ocsf-mapping.json with docs/spec/trace/extend_ocsf.md by placing Provenance objects under unmapped.acs.provenance as {provenance_id, origin, source_id when present} (required_fields: ["provenance_id", "origin"], optional_fields: ["source_id"]), adds acs.provenance.ids to specification/v0.1.0/trace/otel-mapping.json (provenance_attributes.required), and defines deterministic ordering (argument keys in code-point order, outputs by index, deduplicated at first occurrence by provenance_id) across the Trace docs, exporter, conformance checker, and sample trace so cited_provenance_ids on a decision event resolves to its provenance facts on the step event.

Which issue does this implement

Fixes #260
Fixes #262

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

Specification changes

  • I opened a Discussion before this PR
  • Schema changes validate against the JSON Schema spec
  • I described the impact on downstream implementers below

Breaking for implementers? Yes — an exporter claiming ACS-Trace conformance must now emit acs.provenance.ids alongside acs.provenance.origin, the provenance arrays are emitted in deterministic order (arguments by key in code-point order, then outputs by index, with a repeated provenance_id listed once), and required_enrichments / optional_enrichments in ocsf-mapping.json are renamed to required_fields / optional_fields under unmapped.acs.provenance.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

@github-actions

Copy link
Copy Markdown

This pull request's description says it closes #260 #262, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

@acs-board-reconciler acs-board-reconciler Bot moved this from Needs triage to In progress in ACS Project Tracker Oct 28, 2026
@drk-ram
drk-ram force-pushed the spec/trace-provenance-join-260-262 branch 4 times, most recently from ba57a6d to 821c0b6 Compare October 29, 2026 14:40
…acs.provenance.ids

Update ocsf-mapping.json provenance_metadata to place Provenance objects
under unmapped.acs.provenance as {provenance_id, origin, source_id when
present}, add acs.provenance.ids to otel-mapping.json provenance_attributes,
and define deterministic ordering (argument keys in code-point order,
outputs by index, deduplicated by provenance_id) so cited_provenance_ids
on a decision event joins to its provenance facts on the step event.

Fixes GenAI-Security-Project#260
Fixes GenAI-Security-Project#262

Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>

@rocklambros rocklambros left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raghuvar, your mapping change fixes the contradiction in issue #260. The old enrichments shape still fails the OCSF 1.9.0 validator with the two errors the issue lists, and your shape matches provenance.json. You regenerated the sample trace, which is the right way, and it matches what the generator writes today. Every suite passes on integration 19b1197: pytest 924 passed and 1 skipped, and 1287 Bun tests on 1.3.11 and 1.3.14.

This PR can't merge yet. It needs answers from the leads, and it has to land after PR #229.

@bar-capsule @afogel, I need your answers on issue #260 (where Provenance lives) and issue #262 (the three yes or no calls). I'm not repeating them, and this PR doesn't decide them. Ariel's issue #228 answers settle the padding and the minor bump. Running the PR showed the points below, which your answers need to cover. The last one is a new question.

  1. If acs.provenance.ids is required, does provenance_id get minLength: 1? Today "" passes provenance.json, and "" is also the padding value. On issue #228, Ariel asked for minLength: 1 on source_id, and provenance_id didn't get it.

  2. JavaScript's .sort() compares UTF-16 code units, so Bun puts a U+1F600 key before a U+FF5E key, and Python and Go do the opposite. RFC 8785, which ACS uses for hashing, also sorts by UTF-16 code units. If the rule stays code-point order, ask 1 fixes the code. If you pick UTF-16 order, the text changes. I expect this to be rare, because tool argument names are usually ASCII.

    Also, 14 hook schemas carry Provenance, and the rule names only arguments and outputs. Issue #262 said "list fields by index". knowledgeRetrieval and memoryContextRetrieval carry a query and results on one payload, so those need an order too. PR #359 from Eli Sherman already adds content and parser_provenance to this function.

  3. The PR compares provenance_id only, so if two different objects share an id, the second is dropped. I sent argument a as p1 with user_input and argument b as p1 with external. The span showed ["p1"] and ["user_input"]. Integration shows both origins today, and the argument names decide which one stays.

    §7 says the id is unique in a session, and nothing enforces that. A sender that lies can already write any origin, so this is no new attack. It matters when a framework reuses an id by mistake.

    Does "reached twice" mean the same object or the same id? For the same object, comparing provenance_id, origin and source_id keeps both, and the reader sees ["p1", "p1"] with two origins. I ran that, and the exporter and sample tests pass.

  4. Which span does a cited id join to? The PR says "directly" to the step span (events.md:63, events.md:120), and it removes the sentence in requirement 3 (events.md:130) saying this join needs Guardian state. A decision can cite ids from earlier steps, though.

    I ran the FIDES deny from ACS_in_action_example.md through the exporter, with the retrieval as an earlier tool call. It cites p2, p4 and p5, and none of them is on the gated step's span. p2 is on the earlier span. p4 and p5 are on no span, because derived_from isn't on the trace. On extend_ocsf.md, the deny cites p1, and p1 is on a different request. The reference Guardian also stops at 64 objects with no marker.

    Should the join look across the session's step spans, and should requirement 3 keep a sentence for ids no span carries? If yes to both, these words fit: "A reader looks up each id in acs.cited_provenance_ids in acs.provenance.ids on the step spans of the same session." and "A cited id that no span carries, such as one that appears only in derived_from, still needs the Guardian's own records." (context-entry.json holds no provenance ids, so I wrote "own records".)

The tool-call example on extend_ocsf.md has one validator error, the missing device, which is issue #235. The deny example has none.

This PR closes issue #260 if your answer keeps unmapped.acs.provenance with no native copy. It closes issue #262 once points 2 and 4 are settled, because today the order rule and the join fall short of what that issue asks. Once you answer, Raghuvar can push once, and one of you can approve and merge after PR #229 lands.

Raghuvar, please wait for those answers and for PR #229, so you only push once. Asks 1 to 3 may change with the answers. Paths start at reference-implementations/agt/ unless they start with docs/, specification/ or tests/, and line numbers are from your current head. I ran all the asks in a scratch copy, on a real rebase over PR #229. The suites pass, apart from the PR #229 test on Bun 1.3.14 that Jason Crittenden already has to fix.

  1. packages/guardian/src/otel-mapping.ts:376: a bare .sort() is UTF-16 order, and your text says code-point order. Add this above provenanceObjects, and change .sort() to .sort(byCodePoint):

    /** Code-point order, the rule in Trace Events. A bare `.sort()` compares UTF-16
     * code units, which puts U+1F600 ahead of U+FF5E. */
    function byCodePoint(a: string, b: string): number {
      const shared = Math.min(a.length, b.length);
      for (let i = 0; i < shared; i += 1) {
        if (a.charCodeAt(i) !== b.charCodeAt(i)) {
          return a.codePointAt(i)! - b.codePointAt(i)!;
        }
      }
      return a.length - b.length;
    }

    This sorts every key of a request, so I timed it at the 1 MiB body limit. The worst case was about 20 ms, against 13 ms for .sort().

    Test: the first test in ask 2 fails without this change.

  2. packages/guardian/test/otel-trace-exporter.test.ts:444: nothing tests your sort or your duplicate check. I removed each one, and every test still passed. Paste this above the test named "carries no provenance attribute when nothing in the payload has one":

    it("lists provenance by argument key in code-point order, whatever order the wire used", async () => {
      const { exporter, spans } = memoryExporter();
      const argument = (id: string) => ({ value: "v", provenance: { provenance_id: id, origin: "system" } });
      const request = makeEnvelope(
        "steps/toolCallRequest",
        {
          tool: { name: "run_shell" },
          arguments: {
            "\u{1F600}": argument("p-U+1F600"),
            "\uFF5E": argument("p-U+FF5E"),
            ab: argument("p-ab"),
            b: argument("p-b"),
            a: argument("p-a"),
          },
        },
        { id: 1 },
      );
      exporter.end(exporter.start(request, "steps/toolCallRequest", TRUSTED), decisionResponse(1, { decision: "allow" }));
      await exporter.flush();
    
      expect(spans()[0]?.attributes["acs.provenance.ids"]).toEqual(["p-a", "p-ab", "p-b", "p-U+FF5E", "p-U+1F600"]);
    });
    
    it("lists outputs by index, and a repeated provenance_id once, at its first occurrence", async () => {
      const { exporter, spans } = memoryExporter();
      const first = { provenance_id: "p-1", origin: "tool_output", source_id: "Bash" };
      const second = { provenance_id: "p-2", origin: "retrieved" };
      const request = makeEnvelope(
        "steps/toolCallResult",
        {
          tool: { name: "Bash" },
          request_id_ref: crypto.randomUUID(),
          status: "success",
          outputs: [
            { type: "text", value: "a", provenance: first },
            { type: "text", value: "b", provenance: second },
            { type: "text", value: "c", provenance: first },
          ],
        },
        { id: 1 },
      );
      exporter.end(exporter.start(request, "steps/toolCallResult", TRUSTED), decisionResponse(1, { decision: "allow" }));
      await exporter.flush();
    
      const attributes = spans()[0]?.attributes;
      expect(attributes?.["acs.provenance.ids"]).toEqual(["p-1", "p-2"]);
      expect(attributes?.["acs.provenance.origin"]).toEqual(["tool_output", "retrieved"]);
      expect(attributes?.["acs.provenance.source_id"]).toEqual(["Bash", ""]);
    });

    At line 399, change "in argument order" to "in argument-key order". In test/sample-trace.test.ts:185, add this line above the source_id check:

    expect((denied["acs.provenance.ids"] as string[]).length).toBe((denied["acs.provenance.origin"] as string[]).length);

    If the leads answer that the order or the duplicate rule changes, change the expected values to match.

  3. These places still describe the old behavior, and an implementer who reads them builds the wrong exporter.

    • docs/spec/trace/extend_opentelemetry.md:23: change "the two Provenance columns" to "the three Provenance columns".
    • docs/spec/trace/extend_opentelemetry.md:266: change "MUST carry acs.provenance.origin and SHOULD carry" to "MUST carry acs.provenance.ids and acs.provenance.origin and SHOULD carry". Today this line contradicts your events.md:63.
    • packages/guardian/README.md:63: change "the span carries acs.provenance.origin as an array, one element per Provenance object in wire order." to "the span carries acs.provenance.ids and acs.provenance.origin as arrays, one element per Provenance object: arguments by key in code-point order, then outputs by index, with a repeated provenance_id listed once." Then change "Both stop at 64 objects." to "All three stop at 64 objects."
    • packages/guardian/src/otel-mapping.ts:352: change "in wire order" to "in the order provenanceObjects gives". At line 355, say that acs.provenance.ids is always complete too, because provenance.json requires provenance_id.
    • specification/v0.1.0/trace/ocsf-mapping.json:177: change "the OCSF event MUST carry" to "the OCSF step event MUST carry", so it matches your events.md:120. The decision example doesn't carry Provenance, and acs_fields doesn't list it.

    Test: uv run mkdocs build --strict, then the tests in ask 2 and ask 4.

  4. tests/test_trace_mapping_defaults.py: your test pins the mapping lists and nothing checks the example against them. OCSF doesn't validate unmapped, so I put junk there and the validator returned 0 errors. Add import re at the top, and add this test at the end:

    def test_ocsf_example_provenance_has_the_fields_the_mapping_names():
        """OCSF does not validate `unmapped`, so nothing else checks this shape."""
        page = (TRACE.parents[2] / "docs" / "spec" / "trace" / "extend_ocsf.md").read_text(encoding="utf-8")
        heading = "## Example: tool call as Process Activity (1007)"
        block = re.search(r"```json\n(.*?)```", page[page.index(heading):], re.S)
        entries = json.loads(block.group(1))["unmapped"]["acs"]["provenance"]
        fields = json.loads((TRACE / "ocsf-mapping.json").read_text(encoding="utf-8"))["properties"]["provenance_metadata"]["properties"]
        required = set(fields["required_fields"]["default"])
        allowed = required | set(fields["optional_fields"]["default"])
        assert entries
        for entry in entries:
            assert required <= set(entry) <= allowed

    Test: delete "provenance_id": "p1", from the example in extend_ocsf.md, and this test fails.

  5. In the PR description, "Breaking for implementers?" says no. Please change it to yes, because an exporter that claims ACS-Trace and doesn't emit acs.provenance.ids stops conforming. Say that the order of the columns changes too, and that required_enrichments and optional_enrichments are renamed. The next release notes need that.

  6. When PR #229 has merged, do this rebase first, and then make the asks above, so the rebase can't lose them:

    git fetch upstream
    git rebase upstream/integration
    

    I wrote upstream for this repository, so use your own name for that remote if it differs. Git stops on docs/spec/trace/extend_opentelemetry.md and samples/trace/acs-trace-sample.jsonl. Both conflicts are generated text, so please don't merge them by hand. In a rebase, --ours means the integration side:

    git checkout --ours -- docs/spec/trace/extend_opentelemetry.md reference-implementations/agt/samples/trace/acs-trace-sample.jsonl
    git add docs/spec/trace/extend_opentelemetry.md reference-implementations/agt/samples/trace/acs-trace-sample.jsonl
    git rebase --continue
    cd reference-implementations/agt
    bun run sample:trace
    

    Then copy the span with acs.request_id 1b6f0c2a-3d4e-4f5a-8b6c-7d8e9f0a1b03 from the new sample into the JSON block under ## Example: a denied tool call, with two-space indents. Make the asks, commit, and push with git push --force-with-lease.

    Test: uv run pytest tests/test_trace_docs_example.py and bun test test/sample-trace.test.ts. Please run both. The first one still passes if you skip bun run sample:trace, and only the second one catches that.

    PR #359 changes provenanceObjects and the sample too. If it lands first, keep both changes and run bun run sample:trace again.

GitHub still won't let me assign you issue #260 or issue #262. Please accept the repository invite and leave a comment on each issue, as I asked on PR #334, because I can't merge until you're assigned. After you push, I'll run all of this again.

@bar-capsule

Copy link
Copy Markdown
Collaborator

@drk-ram, answered on #260 and #262. Go ahead after #229 lands, following Rock's rebase steps, with three changes to his asks: sort in RFC 8785's order (a bare .sort() already does that in JavaScript, so ask 1 goes away), list two different objects that share an id separately, and add the native enrichments copy from #260.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

3 participants