openai / openai/codex-security

Deferred coverage paths bypass canonical repository-relative validation

Open
#557 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
10.8k
Forks
801
Avg merge
1d 8h
Merged PRs (30d)
257

Description

Summary

Canonical repository paths are constrained throughout the completed-scan contract, but coverage.deferred[].paths is currently only an array of non-empty strings.

A sealed coverage document can therefore describe deferred in-scope work using traversal, absolute, Windows-style, colon-bearing, standalone-dot, or NUL-bearing paths even though manifest scope paths, finding locations, artifact paths, and receipt references reject those forms.

Reproduction / evidence

Current upstream main at 37bf87a692fc72d41f7312cc48808d699d204fba documents each deferred unit as a stable id/reason with optional paths or surfaceIds.

The shared coverage.schema.json defines each deferred path as only:

{
  "type": "string",
  "minLength": 1
}

_validate_coverage() in finalize_scan_contract.py validates surface receipt paths, but does not run deferred paths through _require_safe_relative_path() before the shared schema validation.

A deterministic sealed-contract reproduction is:

  1. copy the bundled examples/completed-scan fixture;
  2. set coverage completeness to partial and add a deferred unit with paths: ["../../outside.ts"];
  3. update the sealed coverage.json digest in scan-manifest.json;
  4. call loadContract().

Current behavior: the traversal path passes the coverage schema and the sealed bundle loads.

Expected behavior: documented deferred source paths should obey the same repository-relative POSIX boundary as other canonical source paths.

Root cause

The deferred-path field was added to the coverage schema without the safe-path constraint already used for canonical source/path-bearing fields.

Suggested fix

Apply the canonical safe repository-relative pattern to deferred[].paths, rejecting:

  • absolute paths;
  • .. traversal segments;
  • backslashes / Windows-style paths;
  • colon-bearing paths;
  • standalone .;
  • NUL characters.

Because the coverage schema is consumed by both finalization and the TypeScript contract loader, one schema change can enforce the invariant on both producer and consumer surfaces.

Add a focused sealed-contract regression that reseals a partial coverage fixture after inserting invalid paths and verifies loadContract() rejects them, with a normal repository-relative path as the control.

Impact

This is contract-integrity correctness. Deferred paths do not cause an arbitrary file read by themselves, but a sealed canonical bundle can currently attribute unreviewed work to a path outside the repository path model enforced everywhere else, which can mislead downstream coverage/reporting consumers.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read coverage.schema.json and _validate_coverage() in finalize_scan_contract.py, then trace loadContract() and the bundled examples/completed-scan fixture. Add a sealed-contract regression covering invalid deferred paths and a normal repository-relative control, resealing the partial fixture as described. Done means both producer and TypeScript consumer reject the invalid paths while accepting the control.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, typescript
Domain
security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.