feat(G13): add wasm-compatible byteport-cli subset crate#241
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Legacy Tooling Scan Report
No violations detected. This is a WARN-mode scan. Fix before strict enforcement begins. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
codeant-ai
Bot
added
the
size:L
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| #[arg(long, default_value_t = 0)] | ||
| content_length: u64, |
There was a problem hiding this comment.
Suggestion: content_length defaults to 0, so if callers omit this flag the CLI emits a content-length: 0 upload instruction. For non-empty payloads this will produce invalid upload behavior (header mismatch/signature mismatch) and fail at runtime. Make content_length required or compute/populate it from actual input instead of silently defaulting to zero. [logic error]
Severity Level: Critical 🚨
- ❌ WASM CLI generates upload instructions with zero content-length.
- ❌ Non-empty uploads using defaults fail with HTTP/storage errors.
- ⚠️ Automation pipelines see confusing, hard-to-debug upload failures.Steps of Reproduction ✅
1. In the WASM CLI argument definition at `crates/byteport-cli/src/main.rs:38-57`, the
`CreateUpload` subcommand defines `content_length` with `#[arg(long, default_value_t =
0)]` on line 55 and `content_length: u64` on line 56, meaning that if the
`--content-length` flag is omitted, the parsed value is `0`.
2. `main()` at `crates/byteport-cli/src/main.rs:60-78` passes this `content_length` into
`cmd_transport_create_upload`, which then builds an `UploadRequest` at lines 91-95 with
`content_length` set exactly to the parsed value (zero if the flag was omitted).
3. In `crates/byteport-transport/src/lib.rs:67-80`, `S3UploadTransport::create_upload`
constructs the `UploadInstruction` headers: it inserts `"content-length"` with
`request.content_length.to_string()`, so when the CLI flag is omitted the resulting
instruction always carries `\"content-length\": \"0\"`.
4. Any automation or service that consumes this CLI's JSON and uses the provided headers
for a real (non-empty) upload will send a payload whose actual byte size does not match
the advertised `content-length: 0`, leading to HTTP/storage-layer errors (e.g. invalid
content-length or signature mismatch) for normal non-empty uploads when the caller forgets
or omits `--content-length`.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/byteport-cli/src/main.rs
**Line:** 55:56
**Comment:**
*Logic Error: `content_length` defaults to `0`, so if callers omit this flag the CLI emits a `content-length: 0` upload instruction. For non-empty payloads this will produce invalid upload behavior (header mismatch/signature mismatch) and fail at runtime. Make `content_length` required or compute/populate it from actual input instead of silently defaulting to zero.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| content_type: &str, | ||
| content_length: u64, | ||
| ) { | ||
| let transport = S3UploadTransport::new(endpoint, bucket, None::<&str>); |
There was a problem hiding this comment.
Suggestion: The CLI constructs S3UploadTransport with None for key_prefix, which diverges from the existing desktop integration that uses a namespace prefix. This can generate object keys outside the expected prefix and cause policy-denied uploads (or cross-namespace key collisions) in environments that enforce prefix-scoped S3 permissions. Add a prefix argument (or reuse the same default prefix strategy) so generated upload instructions match existing transport behavior. [api mismatch]
Severity Level: Major ⚠️
- ❌ WASM CLI uploads bypass expected S3 key namespace prefix.
- ⚠️ Possible policy-denied uploads in prefix-scoped S3 buckets.
- ⚠️ Risk of cross-namespace object key collisions.Steps of Reproduction ✅
1. Observe the desktop integration in `frontend/web/src-tauri/src/lib.rs:17-21`, where
`run()` constructs an `S3UploadTransport` as `S3UploadTransport::new(upload_endpoint(),
upload_bucket(), Some("desktop"))`, applying a `"desktop"` key prefix for all uploads.
2. Note that desktop upload requests go through `frontend/web/src-tauri/src/lib.rs:86-103`
(`ipc::create_upload`), which calls `UploadTransport::create_upload` with an
`UploadRequest` whose `object_key` is typically an application-level path (e.g.
`/projects/demo/screenshot.png`), and the transport then normalizes this with the
`"desktop"` prefix (see `crates/byteport-transport/src/lib.rs:48-58`).
3. Compare this with the new WASM CLI flow in `crates/byteport-cli/src/main.rs:60-80`:
`main()` parses CLI args and calls `cmd_transport_create_upload`, which constructs the
transport at `crates/byteport-cli/src/main.rs:89` as `S3UploadTransport::new(endpoint,
bucket, None::<&str>)`, i.e. with **no** `key_prefix`.
4. For the same logical upload (e.g. `--object-key /projects/demo/screenshot.png`),
desktop will produce a URL like
`https://.../byteport-uploads/desktop/projects/demo/screenshot.png` (see test expectation
in `crates/byteport-transport/src/lib.rs:102-123`), while the CLI will produce
`https://.../byteport-uploads/projects/demo/screenshot.png` with no namespace prefix; in
S3 deployments that enforce prefix-scoped permissions or expect the `"desktop"`-style
prefixing convention, these CLI-generated keys fall outside the expected namespace and can
be denied by policy or collide with other producers' keys.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/byteport-cli/src/main.rs
**Line:** 89:89
**Comment:**
*Api Mismatch: The CLI constructs `S3UploadTransport` with `None` for `key_prefix`, which diverges from the existing desktop integration that uses a namespace prefix. This can generate object keys outside the expected prefix and cause policy-denied uploads (or cross-namespace key collisions) in environments that enforce prefix-scoped S3 permissions. Add a prefix argument (or reuse the same default prefix strategy) so generated upload instructions match existing transport behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![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: bff7268b2b
ℹ️ 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".
| # Coverage instrumentation applied only to native (host) builds. | ||
| # Cross-compilation targets (wasm, etc.) are excluded because | ||
| # the profiler runtime is not available outside the host triple. | ||
| [target.x86_64-pc-windows-msvc] |
There was a problem hiding this comment.
On non-Windows hosts this target table never applies: the Rust build/coverage jobs I checked run on ubuntu-latest, while Cargo documents [target.<triple>].rustflags as flags for that specific target triple. This drops the intended -C instrument-coverage repo config for Linux/macOS native coverage runs; use the actual host triple or a cfg target such as cfg(not(target_arch = "wasm32")) instead of the Windows MSVC triple.
Useful? React with 👍 / 👎.
| serde = { version = "1.0", features = ["derive"] } | ||
| serde_json = "1.0" | ||
| thiserror = "2" |
There was a problem hiding this comment.
The new CLI crate never references serde or thiserror in src/main.rs, and this repo's CI runs cargo machete ., which exits non-zero when unused dependencies are found. As added, the Rust unused-dependency gate will flag this package; either remove these direct deps or add a package.metadata.cargo-machete.ignored entry if they are intentionally reserved.
Useful? React with 👍 / 👎.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Reviewed by laguna-m.1-20260312:free · Input: 139.6K · Output: 12.6K · Cached: 2.1M |
1 participant
User description
Summary
Add a WASM-compatible BytePort CLI crate (
byteport-cli) that compiles towasm32-wasip1for compute/infra automation workflows.Context
Epic G13 requires BytePort CLI to compile to WASM for use in lightweight compute/infra environments where native binaries are not available. The existing CLI (
frontend/web/src-tauri) depends on Tauri and native system APIs, making it unsuitable for wasm targets.This PR introduces a new
byteport-clicrate that provides a subset of CLI commands using only pure-Rust dependencies that are compatible withwasm32-wasip1.Changes
byteport-clicrate (crates/byteport-cli/) — a WASM-compatible CLI binary with atransport create-uploadcommand that produces S3 pre-signed upload instructionscrates/byteport-clito workspace members.cargo/config.tomlfix — scoped coverage instrumentation flags to the host target (x86_64-pc-windows-msvc) so cross-compilation to wasm doesn't fail on missingprofiler_builtinsCargo.lock— the previous lockfile had a corruptsharded-slabduplicate entryKey Implementation Details
The CLI uses
clapfor argument parsing and depends onbyteport-transportfor the S3 upload instruction logic. The crate is structured as a standalone binary ([[bin]]) with no lib target, keeping the compile surface minimal. All dependencies (clap,serde,serde_json,thiserror,byteport-transport) have wasm32-wasip1 support.Use Cases
byteport-cliin lightweight compute/infra automation pipelinesTesting
Links
CodeAnt-AI Description
Add a WASM-compatible BytePort CLI subset for upload instruction creation
What Changed
byteportCLI subset that runs in WASM-friendly environments and supportstransport create-uploadImpact
✅ Upload instructions from WASM environments✅ Fewer WASM build failures✅ Easier automation in lightweight compute environments💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.