Skip to content

test(rules): mirror and cover OAI-029 - #173

Closed
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/openai-ts-path-safety
Closed

bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/openai-ts-path-safety

Conversation

@bradAGI

@bradAGI bradAGI commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Engine half of a coordinated pair. Rules half: trustabl/agent-reliability-rules#95, on a branch of the same name, so the rules-sync job resolves the matching pack rather than main. Neither half should merge alone — check-rules-sync.sh fails if they do.

What the pair adds

OAI-006 covers the Python path-safety case; the TypeScript half was missing. OAI-029 mirrors CSDK-012 including its coarse-signal caveat, stated in the explanation: it flags any filesystem write, not only unnormalized paths, because TS path-normalization analysis isn't wired yet. Confidence 0.5 to match.

In an SDK built around guardrails, the point worth stating is that the guardrail story doesn't cover this. OAI-101 concerns input guardrails on the agent — they screen what enters the conversation, not what a tool does with an argument once the model has produced it. A call that reaches execute() has already passed whatever guardrails were configured, so "we have guardrails" isn't an answer to this finding. The rule says so directly.

What this PR does

  1. Mirrors openai_sdk/path_safety.yaml into testdata/rules-fixture/.
  2. Adds a fire case and a silent case to policyRuleCases, as TestPolicyRules_AllRulesCovered requires.

The silent case applies the remediation the fix text prescribes for the common shape — derive the filename server-side rather than accepting a path from the model — instead of merely deleting the write.

Verification

$ RULES_REPO=../trustabl-rules scripts/check-rules-sync.sh
rules fixture is in sync with production (86 files compared)

$ go vet ./internal/rules/
$ go test ./internal/rules/
ok  	github.com/trustabl/trustabl/internal/rules

Engine half of a coordinated pair with trustabl/agent-reliability-rules#95, on a
branch of the same name so the rules-sync job resolves the matching pack
rather than main. Neither half should merge alone — check-rules-sync.sh
fails if they do.

Mirrors openai_sdk/path_safety.yaml into testdata/rules-fixture and adds a
fire and a silent case to policyRuleCases, as
TestPolicyRules_AllRulesCovered requires.

OAI-006 covers the Python path-safety case; the TypeScript half was
missing. The silent case applies the remediation the fix text prescribes
for the common shape — derive the name server-side rather than accepting a
path from the model — instead of merely removing the write.
@jhumel-code

Copy link
Copy Markdown
Collaborator

Thanks @bradAGI. I diffed the fixture hunk here against the YAML in trustabl/agent-reliability-rules#95 and the two are byte-identical, so rules-sync should stay green as long as the halves land together. The test cases are real rather than vacuous: writeFileSync is in the callee set tsHandlerFacts recognizes, so the fire case genuinely sets writes_fs and PredHasWriteCall returns true on the TS branch, while the silent case has no write callee at all so it returns false. Both snippets carry the @openai/agents import that DiscoverTSOpenAITools gates on, which is the usual way these TS cases go quietly wrong.

One correction to the description. The PR says the silent case applies the remediation the fix text prescribes (derive the filename server-side), but the snippet actually drops the filesystem write entirely and stores into a Map. That is in fact the only silent case this rule permits, since any compliant write still trips has_write_call, so the test is right as written and the description just overstates it. While you are in there, the leftover "Save a note to disk for later retrieval" description on an in-memory implementation is worth tidying.

Happy to take this as a pair with the rules PR. New rules also need a rationale doc in trustabl-rulebook, which we can pick up on our side.

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.

2 participants