Skip to content

fix(cli): reject empty web capture record selections - #9802

Open
ahmed-husssain wants to merge 1 commit into
morluto:mainfrom
ahmed-husssain:fix/cli-web-capture-empty-record-selection
Open

ahmed-husssain wants to merge 1 commit into
morluto:mainfrom
ahmed-husssain:fix/cli-web-capture-empty-record-selection

Conversation

@ahmed-husssain

Copy link
Copy Markdown
Contributor

Summary

Fixes #9767

Aligns the inspect_web_network_capture CLI --record option schema with runtime domain validation by requiring at least one record ordinal when record selection is provided (.min(1)).

Problem and expected behavior

The inspect_web_network_capture CLI previously accepted --record with an empty array in its advertised JSON schema (--schema) and argument parser, whereas runtime domain validation (inspectWebNetworkCaptureInputSchema.shape.record_ordinals) requires at least one nonnegative record ordinal when record selection is specified.

With this change, the CLI options schema specifies minItems: 1 on record, rejecting empty record selections at the CLI schema boundary.

Change and scope

  • Added .min(1) to record: z.array(cliIntegerArgument(0)) in src/cli/webNetworkCaptureCommands.ts.
  • Added boundary schema and option parser assertions in tests/boundary/cli/cliDomainFieldSchema.test.ts.

Follow the contribution scope: Ordinary CLI schema alignment; not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: CLI option schema in src/cli/webNetworkCaptureCommands.ts.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: inspect-web-network-capture --schema now publishes minItems: 1 for option record.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details: none.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/cli/cliDomainFieldSchema.test.ts verifies minItems: 1 in advertised --schema and parser rejection of { record: [] }.
  • Base reproduction or other evidence: Before this fix, inspect-web-network-capture --schema lacked minItems: 1 on record.
  • User-visible CLI/MCP output: record option in --schema now advertises "minItems": 1.
  • Remaining proof gaps: none.

Validation performed

  • npm run test:focused -- tests/boundary/cli/cliDomainFieldSchema.test.ts — passed (2 tests passed)
  • npx oxfmt --check src/cli/webNetworkCaptureCommands.ts tests/boundary/cli/cliDomainFieldSchema.test.ts — passed
  • npm run check:fast — passed (typecheck and lint clean)

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Empty --record was never valid at runtime.
  • Real Hopper/Ghidra, browser, or OS coverage: tested on Windows x64.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none.

Author readiness

Human owner: @ahmed-husssain

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

The inspect_web_network_capture CLI accepted --record with an empty array in its schema and option parser, whereas the runtime domain schema (inspectWebNetworkCaptureInputSchema.shape.record_ordinals) requires at least one nonnegative record ordinal when record selection is supplied.

Add .min(1) to the record CLI option schema and assert rejection in both the advertised schema and the option parser.

Fixes morluto#9767
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T09:04:23.127696Z d532864 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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: d5328645c4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

.describe("Fully qualified message type in the descriptor set"),
record: z
.array(cliIntegerArgument(0))
.min(1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Derive record selection from its domain owner

When the domain's record_ordinals contract changes, this locally copied .min(1) can drift again: inspectWebNetworkCaptureInputSchema.shape.record_ordinals already owns the nonempty-selection rule, and the pre-change mismatch was caused by maintaining this second CLI interpretation. Adapt or extract the owning domain field for CLI decimal tokens instead of copying its cardinality so the advertised schema, parser, and service cannot diverge again.

AGENTS.md reference: AGENTS.md:L29-L29

Useful? React with 👍 / 👎.

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

None yet

Development

Successfully merging this pull request may close these issues.

CLI schema: reject empty web capture record selections

1 participant