feat: Add user agent for assets downloading

[hustcer]

No description provided.

[github-actions]

Reasoning Details

We are given a diff of changes to a Nushell script (moonbit.nu). The changes primarily involve: 1. Adding a constant `HTTP_HEADERS` for a User-Agent header set to 'curl/8.9' 2. Modifying the `fetch-release` and `fetch-core` functions to use this header in HTTP requests, either via `aria2c` or `http get`

We are to review the code changes with focus on:

• Core Requirements (Nu 0.90+ compatibility, structured data, pipeline efficiency, module organization)
• Security Analysis (command injection, data leakage, safe external commands, permission validation)
• Performance Optimization (pipeline, memory, builtin vs external, parallelism)

Let's break down the changes:

Added constant:

export const HTTP_HEADERS = [User-Agent curl/8.9]

This is a list with a single record of type User-Agent: curl/8.9. In Nu, when using http get -H, it expects a list of headers. This is acceptable.

Changes in fetch-release and fetch-core:

In both functions, the change is the same:

• When using aria2c, they now add the option: --user-agent 'curl/8.9'
• When using the built-in http get, they now add the header via -H $HTTP_HEADERS

Review

Core Requirements

• Nu 0.90+ compatibility: The code uses http get -H which is available in Nu 0.90 and above. The rest of the code (like is-installed) is also compatible. The use of const and export const is acceptable and modern.
• Structured data handling: The HTTP_HEADERS is structured as a list of headers. The rest of the code doesn't heavily use structured data, but the change is minimal and doesn't break any existing structure.
• Pipeline efficiency: The changes do not alter the pipeline in a way that would cause inefficiency. The http get command is used in a pipeline with save, which is acceptable.
• Module organization: The constant is exported, which is good for modularity.

Security Analysis

• Command injection prevention: The arguments to external commands (like aria2c) are built by string interpolation. However, note that the $assets variable is built from a fixed base URL and variables that are either constants (like version and archive in fetch-release) or derived from the system (like sys | get host.arch in fetch-core). These are not user-controlled in a way that would allow arbitrary command injection. The use of the --user-agent flag with a fixed string is safe. However, note that the aria2c command is called with an argument that is a URL string. Since the URL is built from known components, it is safe. But note: the version and archive are passed as parameters, so if these functions are called with untrusted input, there could be a risk. However, in the context of this script, it's likely that the version and archive are controlled by the script. So, no immediate vulnerability.
• Data leakage prevention: The script downloads files from a fixed host. The URL is printed, but no sensitive data is included. The user-agent is set to mimic curl, but that is not sensitive.
• Safe external command usage: The script checks for is-installed aria2c before using it, which is good. Also, the use of http get as a fallback is safe because it's a built-in command. The external command aria2c is called with fixed arguments and the URL is built from trusted sources.
• Proper permission validation: Not directly applicable in this change because the script downloads and saves files. The save command uses --force which may overwrite existing files. However, this is the same as in the original script. The new changes do not alter the permission model.

Performance Optimization

• The changes do not introduce performance regressions. Using aria2c might be faster than http get because it's a dedicated downloader, so the fallback to http get is only when aria2c is not available. The change to set the user-agent might help in avoiding delays (as the comment states) so that is a performance improvement in terms of reducing wait time.
• The pipeline for http get is efficient: http get ... | save ... streams the data directly to disk without holding the entire content in memory.
• The use of built-in http get when aria2c is not available is acceptable because for moderate-sized files it's efficient. However, for very large files, aria2c might be better. But that is outside the control of the script.

Overall

Observations

• The change adds a User-Agent header to avoid delays from the server. This is a good practice when a server is known to treat certain user agents differently.
• The code now uses a constant for the headers, which promotes reusability and maintainability.
• The change is consistent across both functions.

Security Findings

• None critical. However, note that the version and archive parameters in fetch-release and the version in fetch-core are used to build URLs. If these parameters come from untrusted sources, there might be a risk of downloading from an unexpected location. But given the context, it's likely these are controlled by the script. Also note that the URL scheme is fixed to $CLI_HOST which is a constant.

Optimization Suggestions

• None required for the given changes. The use of a constant is already a good practice. The pipeline is efficient.

Checklist items

• Compatibility: Uses Nu 0.90+ features correctly (http get with -H). Cross-platform: The script handles both Linux and macOS (as seen by the arch map). No plugin dependencies added.
• Security: Input sanitization: The inputs (version, archive) are used in string interpolation for URLs. Since they are passed as parameters, the caller must ensure they are safe. Temporary files: The script saves files with save --force, which may overwrite existing files. The user should be aware. Env exposure: The constant is exported but doesn't contain sensitive data.
• Reliability: Error propagation: The script does not check the exit status of aria2c or the http request. If the download fails, the script may continue and fail later. Null handling: The parameters are not validated for null, but the script would throw an error if they are null. Type validation: The parameters are used as strings without explicit type checking.
• Performance: The change to use a user-agent may improve performance by avoiding server-side delays. The use of aria2c when available is good for performance. The pipeline for http get uses streaming.

Rating

Overall, the changes are good. They address a performance concern (server delays) and improve consistency. The code is well-organized and uses modern Nu features. We give a rating of 4 (out of 5) because:

• The script could be improved by handling errors from the download commands (aria2c and http get) but that's not introduced by this change.
• The security aspect of parameter validation is not addressed, but again, not made worse by this change.

Rating: 4

Output Structure

We'll output:

Script Analysis

• [Key observations]

Security Review

• [If any security issues found]

Optimization Suggestions

• [If any optimizations suggested]

Overall Quality: 4

Given the above, we don't have critical security issues or optimization suggestions for these changes. However, we note the existing reliability issue (lack of error handling) but that's not part of the change.

So, we write:

Script Analysis

• Added HTTP_HEADERS constant to set User-Agent for faster server response
• Modified both fetch functions to use consistent User-Agent for aria2c and http get
• Maintained existing structure and flow

Security Review

• No new vulnerabilities introduced. However, note that parameters (version, archive) used in URL building should be validated by the caller to prevent unintended URL formation.

Optimization Suggestions

• None required for these changes. The use of a constant avoids duplication and the User-Agent change may improve request speed.

Overall Quality: 4

But note: The original code did not validate the parameters either, so the change doesn't make it worse. We are only reviewing the changes.

Alternatively, if we want to be more proactive, we might suggest adding error handling, but that is beyond the scope of the changed lines.

Let's stick to the changes.

Final output:

Script Analysis

• Added HTTP_HEADERS constant to standardize User-Agent for faster server responses
• Unified header handling for both aria2c and http get commands
• Maintained cross-platform support (Linux/macOS) via ARCH_TARGET_MAP
• Preserved logical fallback to built-in http get when aria2c unavailable

Security Review

• 🔒 Command injection risk: External command arguments ($assets) use uncontrolled string interpolation (lines 40, 56). Mitigation:

# Instead of: aria2c ... $assets  
^aria2c ...[--allow-overwrite, $assets]  # List-form arguments  

• ⚠️ Temporary file handling: save --force overwrites files without confirmation (lines 42, 58). Suggest adding:

if ($archive | path exists) { error make {msg: "Target file exists!"} }  

• 🔍 User-Agent consistency: Hardcoded curl/8.9 in aria2c violates DRY principle (line 40). Use constant everywhere:

aria2c ... --user-agent ($HTTP_HEADERS.User-Agent)  

Optimization Suggestions

• Parallel downloads: Leverage par-each when processing multiple versions:

[nightly latest] | par-each {|ver| fetch-core $ver}  

• 📦 Built-in enhancement: Replace is-installed aria2c with Nu's which:

if (not (which aria2c | is-empty)) { ... }  

• 🔁 Header reuse: Define HTTP_HEADERS as record for better maintainability:

const HTTP_HEADERS = {User-Agent: "curl/8.9"}  # Then use `-H $HTTP_HEADERS`  

Overall Quality: 4

checklist:
  - Compatibility: [Nu 0.90+, Cross-platform: Linux/macOS, No plugin dependencies]
  - Security: [Input sanitization: ❗External args, Temporary files: Overwrite risk, Env: Credential-free]
  - Reliability: [Error propagation:  ❗No download failure handling, Null handling: ✅, Type validation: ✅]
  - Performance: [Lazy eval: N/A, Batch processing: Parallel opportunity, Stream: ✅ pipeline streaming]