Add download command for browser-accessible resources#5
Conversation
|
@codex review this 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: 37a017a4d6
ℹ️ 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".
| } | ||
|
|
||
| try { | ||
| const response = await fetch(url); |
There was a problem hiding this comment.
The download request uses plain fetch(url) from the extension service worker, which defaults to credentials: "same-origin"; because the request origin is the extension, cookies for normal http(s) targets are not sent. In practice, authenticated resources (the main reason for this feature) will often return 401/redirect even with a live browser session. Please fetch with explicit credentials (and related request context as needed) so session-auth downloads work reliably.
Useful? React with 👍 / 👎.
|
|
||
| let out_path = match output { | ||
| Some(p) => PathBuf::from(p), | ||
| None => PathBuf::from(filename), |
There was a problem hiding this comment.
When --output is not provided, the code writes directly to PathBuf::from(filename) where filename comes from remote Content-Disposition/URL data. A malicious filename like ../../target, /absolute/path, or percent-encoded separators can cause writes outside the working directory and overwrite unintended files. The auto filename should be reduced to a safe basename and path traversal components should be rejected.
Useful? React with 👍 / 👎.
| return { | ||
| id: req.id, | ||
| ok: true, | ||
| data: { data, filename, content_type: contentType, size }, |
There was a problem hiding this comment.
This returns the entire base64 file in one native-messaging response object, but Chrome native messaging has a per-message size limit (64 MiB to the host). Because base64 expands payload size, larger downloads will fail once the encoded JSON exceeds that cap. The payload should be streamed/chunked (similar to page chunks) rather than sent in a single response.
Useful? React with 👍 / 👎.
|
I checked this branch locally, and there is still one blocking issue here before merge. Blocking issue: the CLI help text and the PR description both promise support for That means:
I suggest reusing the existing click/type resolution path for
|
Implement a `download` subcommand that fetches a browser-accessible resource using the session's cookies/auth and saves it locally. - Add DOWNLOAD action constant to protocol messages - Add Download CLI subcommand with session_id, target, output, json args - Add download handler in commands.rs: sends request, decodes base64, writes file - Add handleDownload in background service-worker: resolves URL (direct or via element ref), fetches with session cookies, returns base64-encoded content - Add resolve_url content script handler to resolve element src/href to absolute URL - Add 'resolve_url' to ContentRequest type union - Add base64 crate dependency for decoding in Rust Closes #3
…nk download payloads
4fuu
force-pushed
the
feature/download-command
branch
from
099b1c8 to
a78c46b
Compare
1 participant
Summary
browser-cli download <session-id> <target> [--output <path>]command that downloads browser-accessible images and files using the session's cookies/authe3) which resolves itssrc/href, or a direct URLChanges
src/protocol/messages.rs: AddedDOWNLOADaction constantsrc/main.rs: AddedDownloadvariant to CLI command enum with dispatchsrc/cli/commands.rs: Addeddownload()handler (sends request, decodes base64, writes file)extension/src/background/service-worker.ts: AddedhandleDownload()(resolves target URL, fetches with session cookies, returns base64 data + metadata)extension/src/content/content-script.ts: AddedhandleResolveUrl()to resolve element ref IDs to absolute URLsextension/src/shared/types.ts: Addedresolve_urltoContentRequesttype unionCargo.toml: Addedbase64crate dependencyTest plan
browser-cli download <session> <url>downloads a file from a direct URLbrowser-cli download <session> e3resolves element src/href and downloads--outputflag saves to custom path--jsonoutputs structured JSON with path, content_type, sizeCloses #3