Skip to content

fix: validate dynamicTool input before execution - #594

Merged
ariskemper merged 5 commits into
mainfrom
security/h16-dynamic-tool-validation
Mar 11, 2026
Merged

fix: validate dynamicTool input before execution#594
ariskemper merged 5 commits into
mainfrom
security/h16-dynamic-tool-validation

Conversation

@ariskemper

@ariskemper ariskemper commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add input validation to dynamicTool() before calling config.execute()
  • If inputSchema has a parse method (Zod schema), use it for validation
  • Otherwise require input to be a non-null object
  • Prevents unchecked/malformed input from reaching tool implementations

Test plan

  • Lint passes
  • Full CI suite

Parse input through inputSchema if available, otherwise require a
non-null object. Prevents passing unchecked input to tool execute.
@ariskemper
ariskemper marked this pull request as draft March 10, 2026 20:29
@ariskemper
ariskemper marked this pull request as ready for review March 11, 2026 15:50
@ariskemper ariskemper changed the title fix: validate dynamicTool input before execution (H16) fix: validate dynamicTool input before execution Mar 11, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc5ff4390c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tool/factory.ts Outdated
Cover the new runtime validation added to dynamicTool:
- Zod schema validates and rejects input
- Non-schema fallback accepts objects, rejects null/undefined/primitives
MCP tools called without arguments pass undefined input, which the
validation guard rejected. Treat undefined as {} to support zero-input
tools while still rejecting null and primitives.
@ariskemper
ariskemper merged commit 077defa into main Mar 11, 2026
14 checks passed
@ariskemper
ariskemper deleted the security/h16-dynamic-tool-validation branch March 11, 2026 17:46
veryfront-code-agent Bot added a commit that referenced this pull request Aug 21, 2026
Module path-cache publication remains fire-and-forget for render and module-load callers, but each background save now enters a tracked lifecycle before it can settle. Tests and controlled shutdown can drain pending saves, including saves started while a prior drain is in progress.

Constraint: Production rendering must not wait for cache persistence.

Rejected: Await cache writes from persistTransformedModule | puts disk publication back on the critical path.

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep path-cache save failures non-fatal and observable through the existing debug log.

Issue: #594

Tested: deno fmt src/rendering/orchestrator/module-loader/module-persistence.ts src/rendering/orchestrator/module-loader/module-persistence.test.ts; deno task lint; deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts; for run in {1..20}; do deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts >/tmp/module-persistence-run-$run.log || { cat /tmp/module-persistence-run-$run.log; exit 1; }; done; deno task typecheck; git diff --check

Not-tested: deno task test:unit failed in src/proxy/oauth-client.test.ts because the local network returned connection refused before the test timeout path.
veryfront-code-agent Bot added a commit that referenced this pull request Aug 21, 2026
Module path-cache publication remains fire-and-forget for render and module-load callers, but each background save now enters a tracked lifecycle before it can settle. Tests and controlled shutdown can drain pending saves, including saves started while a prior drain is in progress.

Constraint: Production rendering must not wait for cache persistence.

Rejected: Await cache writes from persistTransformedModule | puts disk publication back on the critical path.

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep path-cache save failures non-fatal and observable through the existing debug log.

Issue: #594

Tested: deno fmt src/rendering/orchestrator/module-loader/module-persistence.ts src/rendering/orchestrator/module-loader/module-persistence.test.ts; deno task lint; deno check src/rendering/orchestrator/module-loader/module-persistence.test.ts; deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts; for run in {1..20}; do deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts >/tmp/module-persistence-run-$run.log || { cat /tmp/module-persistence-run-$run.log; exit 1; }; done; deno task typecheck; git diff --check

Not-tested: deno task test:unit failed in src/proxy/oauth-client.test.ts because the local network returned connection refused before the test timeout path.
veryfront-code-agent Bot added a commit that referenced this pull request Aug 21, 2026
Module path-cache publication remains fire-and-forget for render and module-load callers, but each background save now enters a tracked lifecycle before it can settle. Tests and controlled shutdown can drain pending saves, including saves started while a prior drain is in progress.

Constraint: Production rendering must not wait for cache persistence.

Rejected: Await cache writes from persistTransformedModule | puts disk publication back on the critical path.

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep path-cache save failures non-fatal and observable through the existing debug log.

Issue: #594

Tested: deno fmt src/rendering/orchestrator/module-loader/module-persistence.ts src/rendering/orchestrator/module-loader/module-persistence.test.ts; deno task lint; deno check src/rendering/orchestrator/module-loader/module-persistence.test.ts; deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts; for run in {1..20}; do deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/module-persistence.test.ts >/tmp/module-persistence-run-$run.log || { cat /tmp/module-persistence-run-$run.log; exit 1; }; done; deno task typecheck; git diff --check

Not-tested: deno task test:unit failed in src/proxy/oauth-client.test.ts because the local network returned connection refused before the test timeout path.
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.

1 participant