Skip to content

fix(tier3): decode native Codex exec calls - #111

Open
Tomauskasz wants to merge 8 commits into
NVIDIA:mainfrom
Tomauskasz:fix/codex-exec-trajectory
Open

fix(tier3): decode native Codex exec calls#111
Tomauskasz wants to merge 8 commits into
NVIDIA:mainfrom
Tomauskasz:fix/codex-exec-trajectory

Conversation

@Tomauskasz

@Tomauskasz Tomauskasz commented Aug 27, 2026

Copy link
Copy Markdown

Summary

Normalize native Codex custom exec trajectories before Tier 3 deterministic checks and evidence compilation.

Codex records native tool calls inside JavaScript in exec.arguments.input. This change adds one dependency-free parser shared by package evaluations and the copied standalone Harbor verifier. The adapter stages the parser and shared evidence helper beside the verifier template.

The normalizer preserves execution uncertainty instead of manufacturing successful inner calls:

  • unsupported string-input wrappers remain atomic and receive normalization_status = "unsupported_native_codex_exec_wrapper";
  • unsupported wrappers remain uninterpreted, but their opaque outer observation is still scanned for secret exposure;
  • caller-supplied _atif_* fields are removed before normalization so raw input cannot forge internal status or ownership metadata;
  • an inner call with no rendered observation is unobserved_inner_call;
  • one uniquely rendered inner call owns the outer observation;
  • multiple distinct rendered inner calls remain ambiguous;
  • repeated rendering of the same inner call has one stable owner;
  • generic non-Codex exec calls without string arguments.input remain unchanged.

Parser work is bounded to 64 KiB of source, 64 nested JSON containers, 256 statements, and 128 inner tool calls. Invalid JSON resource states and inputs beyond those ceilings fail closed as unsupported wrappers before unbounded signature scans.

Every normalized inner call receives a stable evidence_id. One shared identity rule uses evidence_id, then json_pointer, then path after trimming each candidate. Deduplication, aggregate and Markdown reports, Harbor rendering, suggestion prompts and lookup, and the standalone verifier use that rule while raw json_pointer values continue to reference the real outer ATIF call.

Unsupported execution evidence fails closed across activation, script, workflow-order, negative-case, routing, tool-efficiency, error-recovery, and security checks. Regression coverage includes supported, unsupported, dead, malformed, mixed, rendered, unrendered, ambiguous, repeated-render, generic non-Codex, parser-limit, private-metadata collision, evidence-cardinality, path-only, whitespace-fallback, and public-metadata cases. Arbitrary JavaScript is not scanned as shell text.

Fixes #110.

Verification

  • Focused normalization and suggestion tests: 99 passed

  • Full suite after updating from main: 5562 passed, 17 skipped, 4 deselected

  • Lint: All checks passed

  • Build: source archive and wheel built successfully; the wheel includes codex_tool_call_normalizer.py and evidence.py

  • git diff --check: passed

  • Independent follow-up review: no findings

  • I am familiar with the Contributing Guidelines

  • Added or updated focused tests

  • Updated documentation for user-visible changes

  • Ran make lint

  • Ran the full test suite

  • Ran make build

  • Did not add credentials, private datasets, or proprietary benchmark content

Release Impact

  • No user-visible release note needed
  • Updated CHANGELOG.md

Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
@Tomauskasz
Tomauskasz force-pushed the fix/codex-exec-trajectory branch from 766936e to c33cce5 Compare August 27, 2026 13:06
Context:
Native Codex exec wrappers can contain unsupported, unobserved, or ambiguous inner tool calls. Normalize those calls before Tier 3 checks so collectors do not treat missing execution evidence as a successful inner call.

Changes:
- Add one dependency-free Codex tool-call normalizer for the package evaluator and copied Harbor verifier.
- Keep unsupported wrappers atomic with an explicit unsupported_native_codex_exec_wrapper status, and assign wrapper observations only to uniquely rendered inner calls.
- Add stable inner evidence_id values for deduplication, report lookup, Rich rendering, and suggestion grounding.
- Reject unsupported execution evidence across activation, script, workflow, negative-case, routing, efficiency, recovery, and security checks while preserving generic non-Codex exec calls.
- Expand regression coverage and record the release impact in the changelog.

Impact:
Tier 3 now distinguishes observed inner calls from unobserved, ambiguous, and unsupported native Codex wrappers. Reports and suggestions reference the correct normalized evidence, and the standalone verifier uses the same parser as package evaluations.

Validation:
- Focused tests: 78 passed.
- Tier 3 suite: 664 passed.
- Full suite: 5222 passed, 17 skipped, 4 deselected.
- Lint: all checks passed.
- Build: created the source archive and wheel, including codex_tool_call_normalizer.py.
- git diff --check: passed.

Notes:
Generic exec calls without a string arguments.input remain unchanged for non-Codex compatibility. This repository has no PR CI configured.

Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
@Tomauskasz

Copy link
Copy Markdown
Author

End-to-end follow-up:

  • Installed this branch together with the separate SkillSpector completeness fix and a Harbor 0.13.2 provenance backport.
  • Ran one four-case paired Codex/Terra evaluation.
  • Result: execution_status: succeeded, report_status: complete, 8 expected attempts, 8 scored attempts, and no execution errors.
  • Four retained raw trajectories contained native function_name: "exec" wrappers. The reward artifacts contained four normalized exec_command: evidence records and zero unsupported_native_codex_exec_wrapper statuses.
  • All three tiers passed. With-skill score was 0.9471, baseline was 0.6613, and lift was +0.2858.

This closes the installed-runtime proof gap for the PR. No additional PR change was required.

@rng1995 rng1995 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.

The Codex normalization needs one compatibility guard before this can merge.

arguments = tool_call.get("arguments") or {}
if not isinstance(arguments, dict) or not isinstance(arguments.get("input"), str):
return [tool_call]
parsed = _static_codex_tool_calls(arguments["input"])

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.

This classifies every tool named exec with a string arguments.input as native Codex. A generic ATIF call such as {"function_name":"exec","arguments":{"input":"list repository files"}} is now marked unsupported and makes deterministic checks unscorable. Gate this on Codex provenance or a definitive wrapper signature; otherwise preserve the generic call atomically, with a regression.

@Tomauskasz Tomauskasz Sep 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in d102471. Generic ATIF exec calls with string input now remain atomic unless parsing succeeds or the input carries a definitive Codex wrapper signature: a valid first-line // @exec: pragma or a tools wrapper reference. Malformed signed wrappers still fail closed. The exact reviewer case passes in both the shared and packaged verifier extractors.

Verification: exact regression 2 passed; full normalization module 65 passed; narrow downstream Tier 3 consumers 81 passed; full suite 5,312 passed, 17 skipped, and 4 deselected; full Ruff passed; source and wheel builds passed; git diff --check passed.

Fresh CI, DCO, and Security runs were created, but GitHub marked them action_required before running. The contributor account cannot approve fork workflows, so they are waiting for maintainer approval.

@rng1995

rng1995 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@Tomauskasz Gentle ping when you have a chance: the remaining compatibility review thread is still waiting for an update or response. Thank you for your patience throughout the review and for your contribution.

@Tomauskasz

Copy link
Copy Markdown
Author

@Tomauskasz Gentle ping when you have a chance: the remaining compatibility review thread is still waiting for an update or response. Thank you for your patience throughout the review and for your contribution.

Yeah okay, I'll get back to this soon when I can

Context:
The normalizer treated every `exec` call with string `arguments.input` as native Codex evidence. ATIF permits generic input payloads, and the normalizer does not receive trajectory-level agent provenance.

Changes:
- Preserve unparsable `exec` input atomically unless the source contains a first-line Codex pragma or a `tools` wrapper reference.
- Reuse the pragma pattern in parsing and signature detection.
- Add the generic ATIF input regression to the shared and packaged verifier extractors.

Impact:
Generic `exec` input remains scorable as its original atomic call. Malformed or unsupported inputs with a native Codex signature continue to fail closed.

Validation:
- Exact generic-input regression: 2 passed.
- Full Codex normalization test module: 65 passed.
- Narrow downstream Tier 3 consumer tests: 81 passed.
- Ruff lint and format checks: passed.
- `git diff --check`: passed.

Notes:
The full repository suite and a captured real Codex trajectory replay were not run locally.

Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
Context:
Generic ATIF exec input containing bare prose such as a sentence-ending `tools.` was still classified as an unsupported native Codex wrapper. Require a complete tool-member call signature before applying the fail-closed native-wrapper status.

Changes:
- Match direct and quoted computed-property tool members only when followed by call syntax.
- Preserve the existing escaped-parenthesis signature used by regex-literal wrapper evidence.
- Extend the shared and packaged extractor regression matrix with sentence-ending and property-name prose.

Impact:
Generic exec calls remain atomic when their input only mentions tools in prose. Valid native Codex calls and malformed inputs containing complete wrapper signatures retain their existing normalization behavior.

Validation:
- `uv run pytest -q tests/tier3/test_codex_tool_call_normalization.py`: 69 passed.
- `uv run pytest -q`: 5316 passed, 17 skipped, 4 deselected.
- `uv run ruff check src tests`: passed.
- `uv build`: source archive and wheel built successfully.
- `git diff --check`: passed.

Notes:
The repository-wide formatter check remains red on pre-existing files and is not part of the configured Makefile lint target.
@Tomauskasz

Copy link
Copy Markdown
Author

Follow-up from the complete three-lane review is in 734f8e7.

The definitive Codex-signature guard now requires a tool member followed by call syntax. Generic ATIF prose such as list repository files with the available tools. and describe tools.exec_command before using it remains atomic, while valid direct/computed calls and malformed inputs containing complete wrapper signatures still fail closed.

Verification:

  • exact regression failed in both extractors before the fix;
  • complete normalization module: 69 passed;
  • full suite: 5,316 passed, 17 skipped, 4 deselected;
  • repository Ruff check passed;
  • source and wheel builds passed;
  • git diff --check passed.

Context:
Native Codex exec normalization now bounds parser work, reserves internal ATIF metadata, and preserves opaque outer observations for unsupported wrappers. Tier 3 evidence consumers now resolve one stable identity in evidence_id, json_pointer, then path order.

Changes:
- Cap wrapper input at 64 KiB, object nesting at 64 containers, parsed statements at 256, and inner tool calls at 128.
- Strip caller-supplied _atif_* fields before normalization, centralize normalized traversal and observation ownership, and scan unsupported-wrapper observations for secrets.
- Add a shared evidence-reference identity helper for report rendering, deduplication, Harbor prompts and lookup, and the standalone verifier.
- Copy the normalizer and evidence helper with Harbor verifier tasks and add shared, standalone, packaging, parser-boundary, security, and reporting regressions.

Impact:
Forged normalization metadata cannot suppress destructive-command or secret-exposure findings. Oversized or unsupported wrappers fail closed without interpreting arbitrary JavaScript. Normalized, pointer-only, and path-only evidence references retain stable distinct identities across reports and remediation suggestions.

Validation:
- Full suite: 5,339 passed, 17 skipped, 4 deselected.
- Focused normalization and suggestion suite: 99 passed.
- Ruff lint: passed.
- Source distribution and wheel build: passed.
- Wheel content check: skillevaluator/evidence.py present.
- git diff --check: passed.
- Independent follow-up review: no findings.

Notes:
Unsupported JavaScript remains uninterpreted. Parser ceilings are fixed trust-boundary limits.

Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>
@Tomauskasz

Copy link
Copy Markdown
Author

Additional hardening from the final re-review is in 23f2348.

  • Raw _atif_* fields are removed at the normalization boundary, so input cannot forge unsupported or observation-ownership metadata to bypass security checks.
  • Unsupported wrappers remain uninterpreted, but their opaque outer observation is scanned for secret exposure.
  • Parser work now stops at 64 KiB of source, 64 nested containers, 256 statements, and 128 inner calls; oversized input fails closed before signature regexes run.
  • Evidence identity now consistently falls through evidence_id, json_pointer, and path after trimming each candidate across shared and standalone consumers.

The exact forged-metadata, oversized-prefix, whitespace-fallback, shared-verifier, and standalone-verifier counterexamples now pass. Final verification: 5,339 passed, 17 skipped, 4 deselected; Ruff passed; source and wheel builds passed; git diff --check passed. An independent follow-up review found no remaining issue.

Fresh CI, DCO, and Security runs were created for this head. GitHub marked them action_required; the fork contributor cannot approve those workflows, so they are waiting for maintainer approval.

@Tomauskasz

Copy link
Copy Markdown
Author

Updated the branch from current main using GitHub's clean base-branch merge. The new PR head is 7ab3a13; the feature commit remains 23f2348.

Post-update verification on the exact merged head:

  • Full suite: 5,562 passed, 17 skipped, 4 deselected.
  • Ruff lint: passed.
  • Source distribution and wheel build: passed.
  • Wheel contains both skillevaluator/evidence.py and codex_tool_call_normalizer.py.
  • git diff --check: passed.
  • Local head matches the live PR head and the worktree is clean.

Fresh CI, DCO, and Security runs were created for this merged head. GitHub marked them action_required; they are waiting for NVIDIA maintainer approval of the fork workflows.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Tier 3 ignores native Codex exec trajectories and can miss destructive commands

3 participants