feat: add v1.60 submission archive preflight - #28
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0272677618
ℹ️ 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".
| if ([...paths.keys()].some((existing) => existing.startsWith(`${normalized.path}/`) || normalized.path.startsWith(`${existing}/`))) { | ||
| report.findings.push(archiveFinding("plugin.submission.archive.path_conflict", "Archive entry paths conflict.", { path: normalized.path })); | ||
| continue; |
There was a problem hiding this comment.
When a ZIP contains explicit directory records, this prefix check treats the normal directory/child relationship as a path conflict. Archives produced by common recursive ZIP tools include entries such as pkg/, pkg/.codex-plugin/, and their files, so otherwise valid submission archives are rejected before preflight; only a non-directory parent should conflict with descendants.
Useful? React with 👍 / 👎.
| let rootLayout: SubmissionArchiveRootLayout = "unavailable"; | ||
| let submission: SubmissionPreflightReport | null = null; | ||
|
|
||
| if (!archiveFindings.some((finding) => finding.severity === "fail") && inspection.reader !== null) { |
There was a problem hiding this comment.
When any regular entry uses a well-formed unsupported compression method, inspection deliberately returns no failure but also sets reader to null. This condition then skips root, manifest, asset, and skill validation without adding a blocker, so even an archive with no manifest can report status: "pass", and --require-ready exits 0; unavailable compression coverage should not silently bypass the remaining automatic readiness checks.
Useful? React with 👍 / 👎.
PR Summary by QodoAdd offline Submission Archive Preflight for existing ZIP packages (v1.60)
AI Description
Diagram
High-Level Assessment
Files changed (37)
|
Code Review by Qodo
1. Directory entries reject descendants
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| if ([...paths.keys()].some((existing) => existing.startsWith(`${normalized.path}/`) || normalized.path.startsWith(`${existing}/`))) { | ||
| report.findings.push(archiveFinding("plugin.submission.archive.path_conflict", "Archive entry paths conflict.", { path: normalized.path })); | ||
| continue; |
There was a problem hiding this comment.
1. Directory entries reject descendants 🐞 Bug ≡ Correctness
The prefix-conflict check treats an explicit directory entry such as plugin/ as conflicting with plugin/.codex-plugin/plugin.json, so ordinary ZIPs produced with directory records fail preflight. The documented rule only forbids files from containing children, but the implementation applies it to directories too.
Agent Prompt
## Issue description
Explicit ZIP directory entries are currently treated as conflicting with their children, causing valid archives to fail.
## Issue Context
Only a file/descendant conflict or a file-directory collision should be rejected; a directory and its descendants are valid.
## Fix Focus Areas
- src/core/submission-archive-reader.ts[633-646]
- tests/submission-archive-reader.test.ts[128-140]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (!archiveFindings.some((finding) => finding.severity === "fail") && inspection.reader !== null) { | ||
| const root = discoverRoot(inspection.entries); | ||
| archiveFindings.push(...root.findings); |
There was a problem hiding this comment.
2. Compression suppresses root blockers 🐞 Bug ≡ Correctness
Any well-formed unsupported-compression member leaves inspection.reader null, and buildSubmissionArchivePreflight then skips even path-only root discovery. An archive containing only unsupported.bin therefore reports status: "pass" instead of the automatic missing-root/manifest blocker its already-parsed entry metadata proves.
Agent Prompt
## Issue description
Unsupported compression disables all root and nested structural checks and can make an invalid package pass.
## Issue Context
Root discovery only needs validated entry metadata; content-dependent checks may remain unavailable when no reader can be exposed.
## Fix Focus Areas
- src/core/submission-archive-preflight.ts[262-290]
- src/core/submission-archive-reader.ts[658-679]
- tests/submission-archive-preflight.test.ts[68-125]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (disk !== 0 || centralDisk !== 0 || entriesOnDisk !== entryCount | ||
| || centralStart32 > eocdOffset | ||
| || centralSize32 > eocdOffset - centralStart32) return null; |
There was a problem hiding this comment.
3. Central records can hide 🐞 Bug ⛨ Security
EOCD validation accepts a declared central-directory size smaller than the gap to metadata, while the later walk never verifies its final offset or yielded count against the declared directory extent. A ZIP can therefore declare one visible entry while placing additional central records in the accepted slack, allowing the preflight to pass without inspecting every archive member.
Agent Prompt
## Issue description
The archive parser does not prove that iterated central entries exactly consume the declared central-directory range.
## Issue Context
Validate both classic and ZIP64 directory boundaries and ensure iteration count/end offset agree before exposing a reader.
## Fix Focus Areas
- src/core/submission-archive-reader.ts[382-433]
- src/core/submission-archive-reader.ts[583-592]
- src/core/submission-archive-reader.ts[670-679]
- tests/submission-archive-reader.test.ts[280-310]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if ([...paths.keys()].some((existing) => existing.startsWith(`${normalized.path}/`) || normalized.path.startsWith(`${existing}/`))) { | ||
| report.findings.push(archiveFinding("plugin.submission.archive.path_conflict", "Archive entry paths conflict.", { path: normalized.path })); | ||
| continue; |
There was a problem hiding this comment.
4. Path conflicts scale quadratically 🐞 Bug ➹ Performance
For every entry, the reader materializes all prior keys and scans them with prefix comparisons, making conflict validation O(entries² × path length). With the allowed 5,000 entries and ZIP names up to the format limit, a sub-100-MB adversarial archive can force hundreds of millions of comparisons over long common prefixes and stall the preflight.
Agent Prompt
## Issue description
Path conflict validation performs a full scan of all prior paths for every archive entry.
## Issue Context
The archive permits 5,000 entries and does not impose a byte-length cap on each path, so conflict checks must scale with entries and segments rather than all path pairs.
## Fix Focus Areas
- src/core/submission-archive-reader.ts[202-211]
- src/core/submission-archive-reader.ts[577-646]
- tests/submission-archive-reader.test.ts[128-140]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const signed = descriptor.readUInt32LE(0) === 0x08074b50; | ||
| const base = signed ? 4 : 0; |
There was a problem hiding this comment.
5. Rejects unsigned crc collisions 🐞 Bug ≡ Correctness
An unsigned ZIP data descriptor whose CRC32 is 0x08074b50 is treated as if it carried the optional descriptor signature, shifting both size reads by four bytes. The expected-size comparison then reports descriptor_invalid for an otherwise valid archive.
Agent Prompt
## Issue description
An unsigned ZIP data descriptor may have CRC32 `0x08074b50`, the same value as the optional descriptor signature. The parser uses that first word alone to select the signed layout, then reads the descriptor's size fields from shifted offsets and rejects the entry.
## Issue Context
This preflight is intended to accept well-formed existing ZIP packages, including descriptors without the optional signature.
## Fix Focus Areas
- src/core/submission-archive-reader.ts[349-367]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
Summary
Test Plan