From 45d9aca7fe946347700ce3c982cdafca9e836167 Mon Sep 17 00:00:00 2001 From: Firstp1ck Date: Sat, 8 Aug 2026 09:43:27 +0200 Subject: [PATCH] fix: complete 0.3.0 Pacsea prerequisites --- .github/workflows/rust.yml | 11 + CHANGELOG.md | 7 + Cargo.toml | 10 + README.md | 12 +- .../pacsea-v0-3-0-upstream-prerequisites.html | 126 +++++ src/aur/comments.rs | 158 +++++- src/aur/info.rs | 167 ++++++- src/aur/pkgbuild.rs | 131 ++++- src/deps/srcinfo.rs | 197 +++++++- src/http.rs | 381 ++++++++++++++ src/install/batch.rs | 60 ++- src/install/command.rs | 152 ++++-- src/install/mod.rs | 4 +- src/install/shell.rs | 61 ++- src/lib.rs | 3 + tests/compatibility_contract.rs | 136 ++++- tests/install_integration.rs | 463 +++++++++++++++++- 17 files changed, 1957 insertions(+), 122 deletions(-) create mode 100644 reports/pacsea-v0-3-0-upstream-prerequisites.html create mode 100644 src/http.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index cfe85ba..03df270 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -54,6 +54,7 @@ jobs: - index,fuzzy-search - aur,cache-disk - deps,aur + - aur,deps,index,install,news,sandbox steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable @@ -66,6 +67,16 @@ jobs: else cargo test --no-default-features --features "${{ matrix.features }}" -- --test-threads=1 fi + - name: Verify Pacsea optional-feature isolation + if: matrix.features == 'aur,deps,index,install,news,sandbox' + shell: bash + run: | + cargo tree --locked --no-default-features --features "${{ matrix.features }}" > /tmp/pacsea-tree.txt + if grep -Eq '(^| )fuzzy-matcher v|(^| )dirs v' /tmp/pacsea-tree.txt; then + echo "Pacsea feature projection unexpectedly enables fuzzy-search or cache-disk dependencies" >&2 + cat /tmp/pacsea-tree.txt >&2 + exit 1 + fi msrv: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 0421829..403877f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,12 +15,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - PKGBUILD and `.SRCINFO` sandbox dependency preflight analysis. - Bounded, deterministic dependency graph resolution with injected metadata, cycle diagnostics, constraint intersection, provider provenance, split-package handling, and stable tree rendering. - Deterministic feature-matrix, complexity, missing-tool, and compatibility quality gates. +- Exact no-default-feature Pacsea compatibility projection for `aur,deps,index,install,news,sandbox`, including optional-dependency isolation checks. +- One private streamed HTTP response reader shared by AUR info, comments, PKGBUILD, and `.SRCINFO` retrieval. ### Changed - Selected the next minor version, `0.3.0`, because post-v0.2.0 work adds multiple public feature-gated modules while preserving existing entry points. - Package version comparison now handles epochs and follows libalpm by comparing numeric pkgrel only when both operands declare one. - Existing Arch news and advisory fetch functions now reject responses above 512 KiB with `InputTooLong`; AUR search responses are bounded to 4 MiB without imposing a result-count cap. +- AUR info, comments, PKGBUILD, and `.SRCINFO` responses now enforce strict streamed 10 MiB byte ceilings and reject invalid UTF-8 before parsing. +- Malformed AUR info JSON now returns a contextual `Parse` error instead of an `InfoFailed` transport error; `.SRCINFO` mid-body transport failures likewise return contextual `Parse` errors naming the package. +- Generated install/remove argv and AUR install fallback bodies now place `--` before package operands. +- The missing-AUR-helper fallback now writes the existing message to stderr and exits with status 127 instead of appearing successful. - Release preview now performs local verification without committing, tagging, pushing, creating releases, or publishing. - Internal pacman and helper parsing remains locale-independent through `LC_ALL=C`/`LANG=C`; no unused localized-label API was added. @@ -30,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Environment- and PATH-mutating tests are serialized and restore process state. - The previous complexity script no longer reports success after analyzing zero functions. - Documentation and automation now consistently use `v` tags. +- Install command builders reject package names whose first byte is not a lowercase ASCII letter or digit, preventing leading-option and hidden-name confusion while preserving valid internal punctuation. ### Compatibility and intentional differences diff --git a/Cargo.toml b/Cargo.toml index 0fa5b3e..58d75e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,16 @@ license = "MIT" repository = "https://github.com/Firstp1ck/arch-toolkit" keywords = ["archlinux", "aur", "pacman", "package-manager"] categories = ["api-bindings", "command-line-utilities"] +include = [ + "src/**", + "examples/**", + "tests/**", + "Cargo.toml", + "Cargo.lock", + "README.md", + "CHANGELOG.md", + "LICENSE", +] [features] default = ["aur"] diff --git a/README.md b/README.md index c2745a7..21f3672 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Add `arch-toolkit` to your `Cargo.toml`: ```toml [dependencies] -arch-toolkit = "0.2" +arch-toolkit = "0.3" ``` ### Feature Flags @@ -78,25 +78,25 @@ arch-toolkit = "0.2" To disable default features: ```toml -arch-toolkit = { version = "0.2", default-features = false, features = ["aur"] } +arch-toolkit = { version = "0.3", default-features = false, features = ["aur"] } ``` To enable dependency parsing: ```toml -arch-toolkit = { version = "0.2", features = ["deps"] } +arch-toolkit = { version = "0.3", features = ["deps"] } ``` To enable disk caching: ```toml -arch-toolkit = { version = "0.2", features = ["cache-disk"] } +arch-toolkit = { version = "0.3", features = ["cache-disk"] } ``` To enable package index queries: ```toml -arch-toolkit = { version = "0.2", features = ["index"] } +arch-toolkit = { version = "0.3", features = ["index"] } ``` ## Quick Start @@ -416,7 +416,7 @@ let remove = with_privilege( detect_privilege_tool().expect("sudo or doas required"), build_remove_command(&["old-package"], CascadeMode::CascadeWithConfigs, true)?, ); -println!("{remove}"); // sudo pacman -Rns --noconfirm old-package +println!("{remove}"); // sudo pacman -Rns --noconfirm -- old-package ``` ### News and Security Advisories diff --git a/reports/pacsea-v0-3-0-upstream-prerequisites.html b/reports/pacsea-v0-3-0-upstream-prerequisites.html new file mode 100644 index 0000000..0f60213 --- /dev/null +++ b/reports/pacsea-v0-3-0-upstream-prerequisites.html @@ -0,0 +1,126 @@ + + + + + + +arch-toolkit 0.3.0 — Pacsea prerequisite implementation report + + + + +
+
+Implementation verified +

arch-toolkit 0.3.0 Pacsea prerequisites

+

Evidence-based implementation report for the bounded network, install-safety, compatibility, and crates.io packaging work defined by the canonical Pacsea prerequisite plan.

+
Conclusion: U1–U8 are implemented and pass the complete local quality matrix. The crate payload is lean and mechanically publishable. Publication, tagging, pushing, and GitHub release creation were not performed; the final immutable commit and clean-tree dry run are recorded in the archived plan and branch PR after this report enters the release commit.
+

Confidence: 97/100. Deterministic tests, full feature checks, dependency audit, MSRV, package inspection, two independent providers, and direct source review support the conclusion. Remaining uncertainty is limited to real paru/yay behavior and intentionally ignored live-service diagnostics.

+
+
+ +
+

Overview

+
+
Network4 × 10 MiBAUR info, comments, PKGBUILD, and .SRCINFO streamed ceilings
+
Compatibility6 featuresExact Pacsea projection with defaults disabled
+
Package96 filesLean crates.io payload; plans, reports, CI, and dev scripts excluded
+
Complexity0 violationsAll measured functions below the repository threshold
+
+

Release-readiness map

+
+ + + + +
AreaImplemented contractEvidenceStatus
U1–U4 networkHeader precheck, chunk streaming, strict UTF-8, contextual parse/size errorsShared raw-socket/wiremock tests plus per-operation fixturesVerified
U5–U7 commandsStrict first byte, -- operands, stderr/status 127 fallbackDirect/batch/remove argv tests and isolated fake-helper executionVerified
U8 Pacsea contractaur,deps,index,install,news,sandbox without defaultsModel fixtures, CI matrix row, dependency-tree isolationVerified
U9 release identityLocal release commit and clean dry-run; no external release actionExact SHA and command receipts in archived plan/branch PRLocal handoff only
+
+
+

Implementation map

+
+
W1

Bounded HTTP response reader

src/http.rs rejects declared oversize before allocation, caps streamed chunks using saturating arithmetic, and validates UTF-8 only after the byte ceiling. Four AUR callers retain request, retry, timeout, backoff, and status behavior while adding package/operation context.

+
W2

Install operand safety

Package names now match ^[a-z0-9][a-z0-9@._+-]*$. Install/remove builders and runtime AUR install fallback place -- before operands. Missing helpers emit the unchanged message to stderr and terminate the fallback subshell with status 127.

+
I1

Compatibility and packaging

The exact Pacsea feature set has aggregate model tests and a CI test row. A dependency-tree gate rejects fuzzy/cache leakage. README and changelog describe 0.3 behavior, and the manifest whitelist creates a lean crate.

+
+
+ +Implementation and release evidence flowNetwork responses pass through status handling, a shared bounded reader, strict UTF-8, and operation parsing; integrated code then passes compatibility tests, the full matrix, independent review, and a local release commit. + + +HTTP statusand context +10 MiB streamedbyte ceiling +Strict UTF-8then parsing +Pacsea modelcontracts +Full releasematrix +Two-providerreview +Immutable localhandoff + +
Green-outlined nodes are implemented completion gates. External publication remains intentionally outside the flow.
+
+
Scope invariant: arch-toolkit remains a frontend-agnostic library. Pacsea retains command execution, dry-run presentation, privilege/password sessions, PTY lifecycle, cancellation, caches, UI state, and transaction orchestration.
+
+
+

Testing and acceptance evidence

+
    +
  • Formatting, strict all-feature Clippy, default check, and default serial tests pass.
    • +
  • No-default checks pass for the empty feature set and each of aur, deps, index, install, news, and sandbox.
    • +
  • The exact Pacsea projection passes serially; all features pass serially and in parallel.
    • +
  • Shared HTTP tests cover declared oversize, missing length, dishonest long and short headers, chunked overflow, exact limit, and invalid UTF-8.
    • +
  • Install integration tests cover direct/remove/batch/fallback leading-option rejection, exact argv, privilege boundaries, helper preference, stderr, and status 127.
    • +
  • Complexity self-test/report, rustdoc, RustSec audit, Rust 1.91 MSRV, dependency-tree isolation, package inspection, and publish dry run pass.
    • +
+
+ + + + + +
GateResultImportant observation
Pacsea projectionExit 0No fuzzy-matcher or dirs in the selected dependency tree.
All-feature testsExit 0Serial and parallel runs complete without failures; live host/network tests remain ignored diagnostics.
ComplexityExit 0No function reaches the configured threshold of 25.
RustSec auditExit 0No known vulnerabilities reported for the lockfile.
Package payload96 filesSource, examples, tests, Cargo files, README, changelog, and license only.
+
Representative verification command
cargo test --locked --no-default-features \
+  --features aur,deps,index,install,news,sandbox \
+  -- --test-threads=1
+
+
+

Independent review and dispositions

+

Two fresh-context read-only reviewers from distinct provider families inspected the integrated source and tests. Findings were advisory and independently dispositioned by the integration owner.

+
+ + + + + +
ReviewerFindingDispositionEvidence/action
R1 · AnthropicHTTP helper was compiled but unused under index/news-only builds.Accepted and fixedGate narrowed to feature = "aur"; standalone checks are warning-free.
R1 · AnthropicMalformed info JSON and .SRCINFO mid-body error variants are behavior deltas.Accepted/documentedCHANGELOG explicitly records contextual Parse behavior.
R1 · AnthropicDishonest short Content-Length lacked an explicit test.Accepted and fixedRaw response fixture combines a short declaration with chunked framing and proves streamed overflow rejection.
R1 · AnthropicReal paru/yay -- behavior not executed.Deferred to Pacsea parityFake-helper argv is deterministic; strict name validation remains an independent safety barrier.
R2 · GoogleNo blockers; package and compatibility contracts pass.AcceptedIndependent commands confirmed projection, matrix, complexity, and lean package.
+

Review outcome: PASS after the accepted hygiene and coverage fixes. No unresolved blocker remains.

+
+
+

Pacsea and crates.io handoff

+
    +
  1. Pacsea dependency: use default-features = false with only aur,deps,index,install,news,sandbox; pin the exact local/remote commit recorded in the archived plan until 0.3.0 is published.
    2. +
  2. Expected behavior changes: toolkit-generated install/remove argv contains --; missing helper fallback writes to stderr and exits 127; bounded AUR body failures return contextual size/parse errors.
    3. +
  3. Consumer responsibilities: retain search cap, caches, retry/circuit policy, repository merge/enrichment, article/read state, scanners, AUR voting, execution, dry-run, password/privilege, PTY, lock, confirmation, cancellation, and logging.
    4. +
  4. External release boundary: tagging, pushing, GitHub release creation, and cargo publish require a separate explicit authorization. This implementation run performs none of them.
    5. +
+
Canonical record: Archived prerequisite plan. The archived plan and ignored branch PR record hold the exact release commit and clean-tree command receipts because a report included in that commit cannot self-reference its own final SHA.
+

Residual risks

+
    +
  • Live AUR/Arch endpoints and real host helpers remain intentionally outside deterministic release gates.
    • +
  • Pacsea must rerun its own differential behavior, packaging, and security gates before deleting local implementations.
    • +
  • No crates.io upload has occurred; readiness is not publication.
    • +
+
+

Generated for arch-toolkit 0.3.0 release hardening on 2026-08-08. Evidence sources: repository diff, canonical plan, worker handoffs, command logs, package list, and two independent review artifacts.

+
+ + + diff --git a/src/aur/comments.rs b/src/aur/comments.rs index 51a1bb1..ef04823 100644 --- a/src/aur/comments.rs +++ b/src/aur/comments.rs @@ -14,6 +14,9 @@ use reqwest::header::{ACCEPT, ACCEPT_LANGUAGE, HeaderMap, HeaderValue}; use scraper::{ElementRef, Html, Selector}; use tracing::debug; +/// Maximum accepted AUR comments response body size in bytes. +const MAX_AUR_COMMENTS_RESPONSE_BYTES: usize = 10 * 1024 * 1024; + /// Context for extracting comment data from HTML elements. struct CommentExtractionContext<'a> { /// Parsed HTML document @@ -117,6 +120,7 @@ pub async fn comments(client: &ArchClient, pkgname: &str) -> Result` containing HTML text, or an error. @@ -159,15 +163,14 @@ async fn perform_comments_request( } }; - let html_text = match response.text().await { - Ok(text) => text, - Err(e) => { - debug!(error = %e, pkgname = %pkgname, "failed to read AUR comments response"); - return Err(ArchToolkitError::comments_failed(pkgname, e)); - } - }; - - Ok(html_text) + let resource_label = format!("AUR comments for package '{pkgname}'"); + crate::http::read_bounded_response_text( + response, + MAX_AUR_COMMENTS_RESPONSE_BYTES, + &resource_label, + |error| ArchToolkitError::comments_failed(pkgname, error), + ) + .await } /// What: Parse HTML and extract comments. @@ -190,16 +193,24 @@ fn parse_comments_html(html_text: &str, pkgname: &str) -> Result // - Each comment has an

with author and date // - The content is in a following
with id "comment-{id}-content" // - Pinned comments appear before "Latest Comments" heading - let comment_header_selector = Selector::parse("h4.comment-header").map_err(|e| { - ArchToolkitError::Parse(format!("Failed to parse comment header selector: {e}")) + let comment_header_selector = Selector::parse("h4.comment-header").map_err(|error| { + ArchToolkitError::Parse(format!( + "failed to parse AUR comments for package '{pkgname}' header selector: {error}" + )) })?; - let date_selector = Selector::parse("a.date") - .map_err(|e| ArchToolkitError::Parse(format!("Failed to parse date selector: {e}")))?; + let date_selector = Selector::parse("a.date").map_err(|error| { + ArchToolkitError::Parse(format!( + "failed to parse AUR comments for package '{pkgname}' date selector: {error}" + )) + })?; // Find the "Latest Comments" heading to separate pinned from regular comments - let heading_selector = Selector::parse("h3, h2, h4") - .map_err(|e| ArchToolkitError::Parse(format!("Failed to parse heading selector: {e}")))?; + let heading_selector = Selector::parse("h3, h2, h4").map_err(|error| { + ArchToolkitError::Parse(format!( + "failed to parse AUR comments for package '{pkgname}' heading selector: {error}" + )) + })?; // Check if there's a "Pinned Comments" section let has_pinned_section = document.select(&heading_selector).any(|h| { @@ -723,7 +734,10 @@ fn format_text_node(element: &ElementRef) -> String { #[cfg(test)] mod tests { + use super::*; use crate::error::ArchToolkitError; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; #[test] fn test_comments_error_includes_package_context() { @@ -741,4 +755,118 @@ mod tests { "Error message should indicate comments operation: {error_msg}" ); } + + #[tokio::test] + /// What: Verify an oversized AUR comments body is rejected while reading. + /// + /// Inputs: + /// - A local HTML response one byte above the approved 10 MiB ceiling. + /// + /// Output: + /// - `InputTooLong` retaining comments-operation and package context. + /// + /// Details: + /// - The local fixture exercises the request helper without live AUR traffic. + async fn oversized_aur_comments_response_is_rejected() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/comments")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(vec![ + b'x'; + MAX_AUR_COMMENTS_RESPONSE_BYTES + + 1 + ])) + .mount(&server) + .await; + + let error = perform_comments_request( + &ReqwestClient::new(), + &format!("{}/comments", server.uri()), + "yay", + ) + .await + .expect_err("oversized comments response must fail"); + let message = error.to_string(); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: MAX_AUR_COMMENTS_RESPONSE_BYTES, + .. + } + )); + assert!(message.contains("comments")); + assert!(message.contains("yay")); + } + + #[tokio::test] + /// What: Preserve comments-operation context for status and UTF-8 body failures. + /// + /// Inputs: + /// - Local HTTP 503 and invalid UTF-8 responses for package `yay`. + /// + /// Output: + /// - Contextual errors identifying comments and the package. + /// + /// Details: + /// - Status rejection precedes body reading; UTF-8 validation follows the byte bound. + async fn aur_comments_status_and_utf8_errors_are_contextual() { + for (path_value, template) in [ + ("/status", ResponseTemplate::new(503)), + ( + "/utf8", + ResponseTemplate::new(200).set_body_bytes([0xf0, 0x28, 0x8c]), + ), + ] { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(path_value)) + .respond_with(template) + .mount(&server) + .await; + + let error = perform_comments_request( + &ReqwestClient::new(), + &format!("{}{path_value}", server.uri()), + "yay", + ) + .await + .expect_err("invalid comments response must fail"); + let message = error.to_string(); + + assert!(message.contains("comments")); + assert!(message.contains("yay")); + } + } + + #[tokio::test] + /// What: Return a normal bounded AUR comments HTML fixture unchanged. + /// + /// Inputs: + /// - A small local HTML package page. + /// + /// Output: + /// - The exact UTF-8 response body. + /// + /// Details: + /// - HTML parsing remains a separate, non-executing operation. + async fn normal_aur_comments_fixture_is_read() { + let server = MockServer::start().await; + let html = "

Latest Comments

"; + Mock::given(method("GET")) + .and(path("/comments")) + .respond_with(ResponseTemplate::new(200).set_body_string(html)) + .mount(&server) + .await; + + let body = perform_comments_request( + &ReqwestClient::new(), + &format!("{}/comments", server.uri()), + "yay", + ) + .await + .expect("normal comments fixture"); + + assert_eq!(body, html); + } } diff --git a/src/aur/info.rs b/src/aur/info.rs index a8c4ad3..0620cb3 100644 --- a/src/aur/info.rs +++ b/src/aur/info.rs @@ -13,6 +13,9 @@ use reqwest::Client; use serde_json::Value; use tracing::{debug, warn}; +/// Maximum accepted AUR info response body size in bytes. +const MAX_AUR_INFO_RESPONSE_BYTES: usize = 10 * 1024 * 1024; + /// What: Fetch detailed information for one or more AUR packages. /// /// Inputs: @@ -112,6 +115,7 @@ pub async fn info(client: &ArchClient, names: &[&str]) -> Result>` containing package details, or an error. @@ -146,15 +150,17 @@ async fn perform_info_request( } }; - let json: Value = match response.json().await { - Ok(json) => json, - Err(e) => { - warn!(error = %e, packages = ?package_names, "failed to parse AUR info JSON"); - // reqwest::Error can contain serde_json::Error, but we'll treat it as network error - // since the JSON parsing happens inside reqwest - return Err(ArchToolkitError::info_failed(package_names, e)); - } - }; + let resource_label = format!("AUR info for packages [{}]", package_names.join(", ")); + let text = crate::http::read_bounded_response_text( + response, + MAX_AUR_INFO_RESPONSE_BYTES, + &resource_label, + |error| ArchToolkitError::info_failed(package_names, error), + ) + .await?; + let json: Value = serde_json::from_str(&text).map_err(|error| { + ArchToolkitError::Parse(format!("failed to parse {resource_label} JSON: {error}")) + })?; let mut packages = Vec::new(); @@ -244,6 +250,8 @@ mod tests { use super::*; use crate::error::ArchToolkitError; use serde_json::json; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; #[test] fn test_info_error_includes_package_context() { @@ -266,6 +274,147 @@ mod tests { ); } + #[tokio::test] + /// What: Verify an oversized AUR info body is rejected before JSON parsing. + /// + /// Inputs: + /// - A local response declaring a body one byte above the approved 10 MiB ceiling. + /// + /// Output: + /// - `InputTooLong` retaining AUR info and package context. + /// + /// Details: + /// - This regression test uses a deterministic wiremock endpoint and no live service. + async fn oversized_aur_info_response_is_rejected() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/info")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(vec![ + b'x'; + MAX_AUR_INFO_RESPONSE_BYTES + + 1 + ])) + .mount(&server) + .await; + + let error = + perform_info_request(&Client::new(), &format!("{}/info", server.uri()), &["yay"]) + .await + .expect_err("oversized AUR info response must fail"); + let message = error.to_string(); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: MAX_AUR_INFO_RESPONSE_BYTES, + .. + } + )); + assert!(message.contains("info")); + assert!(message.contains("yay")); + } + + #[tokio::test] + /// What: Preserve operation and package context for empty or malformed AUR info JSON. + /// + /// Inputs: + /// - Local empty and syntactically malformed successful responses. + /// + /// Output: + /// - Contextual `Parse` errors for both bodies. + /// + /// Details: + /// - Explicit serde JSON parsing occurs only after the bounded UTF-8 read. + async fn empty_and_malformed_aur_info_bodies_are_contextual() { + for (path_value, body) in [("/empty", ""), ("/malformed", "{")] { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(path_value)) + .respond_with(ResponseTemplate::new(200).set_body_string(body)) + .mount(&server) + .await; + + let error = perform_info_request( + &Client::new(), + &format!("{}{path_value}", server.uri()), + &["yay"], + ) + .await + .expect_err("invalid AUR info JSON must fail"); + let message = error.to_string(); + + assert!(matches!(error, ArchToolkitError::Parse(_))); + assert!(message.contains("AUR info")); + assert!(message.contains("yay")); + } + } + + #[tokio::test] + /// What: Preserve info-operation context for a non-success status. + /// + /// Inputs: + /// - A local HTTP 503 response for package `yay`. + /// + /// Output: + /// - Existing `InfoFailed` with status and package context. + /// + /// Details: + /// - Status handling remains ahead of bounded body consumption. + async fn non_success_aur_info_status_is_contextual() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/info")) + .respond_with(ResponseTemplate::new(503)) + .mount(&server) + .await; + + let error = + perform_info_request(&Client::new(), &format!("{}/info", server.uri()), &["yay"]) + .await + .expect_err("non-success AUR info status must fail"); + let message = error.to_string(); + + assert!(matches!(error, ArchToolkitError::InfoFailed { .. })); + assert!(message.contains("yay")); + assert!(message.contains("503")); + } + + #[tokio::test] + /// What: Parse a normal bounded AUR info fixture through the request path. + /// + /// Inputs: + /// - One valid AUR RPC result from a local HTTP server. + /// + /// Output: + /// - One populated `AurPackageDetails` entry. + /// + /// Details: + /// - Covers the explicit JSON parser after streamed response reading. + async fn normal_aur_info_fixture_is_parsed() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/info")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "results": [{ + "Name": "yay", + "Version": "12.3.4-1", + "Description": "AUR helper", + "Depends": ["git"] + }] + }))) + .mount(&server) + .await; + + let packages = + perform_info_request(&Client::new(), &format!("{}/info", server.uri()), &["yay"]) + .await + .expect("normal AUR info fixture"); + + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].name, "yay"); + assert_eq!(packages[0].depends, ["git"]); + } + #[test] fn test_info_parses_valid_response() { let json = json!({ diff --git a/src/aur/pkgbuild.rs b/src/aur/pkgbuild.rs index ebee3ba..776d67d 100644 --- a/src/aur/pkgbuild.rs +++ b/src/aur/pkgbuild.rs @@ -19,6 +19,8 @@ use tracing::debug; static PKGBUILD_RATE_LIMITER: Mutex> = Mutex::new(None); /// Minimum interval between PKGBUILD requests in milliseconds. const PKGBUILD_MIN_INTERVAL_MS: u64 = 200; +/// Maximum accepted PKGBUILD response body size in bytes. +const MAX_AUR_PKGBUILD_RESPONSE_BYTES: usize = 10 * 1024 * 1024; /// What: Fetch PKGBUILD content for an AUR package. /// @@ -141,6 +143,7 @@ pub async fn pkgbuild(client: &ArchClient, package: &str) -> Result { /// Inputs: /// - `client`: HTTP client to use for requests. /// - `url`: URL to request. +/// - `package`: Package name retained in every operation error. /// /// Output: /// - `Result` containing PKGBUILD text, or an error. @@ -177,20 +180,22 @@ async fn perform_pkgbuild_request(client: &Client, url: &str, package: &str) -> } }; - let text = match response.text().await { - Ok(text) => text, - Err(e) => { - debug!(error = %e, package = %package, "failed to read PKGBUILD response"); - return Err(ArchToolkitError::pkgbuild_failed(package, e)); - } - }; - - Ok(text) + let resource_label = format!("AUR PKGBUILD for package '{package}'"); + crate::http::read_bounded_response_text( + response, + MAX_AUR_PKGBUILD_RESPONSE_BYTES, + &resource_label, + |error| ArchToolkitError::pkgbuild_failed(package, error), + ) + .await } #[cfg(test)] mod tests { + use super::*; use crate::error::ArchToolkitError; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; #[test] fn test_pkgbuild_error_includes_package_context() { @@ -208,4 +213,112 @@ mod tests { "Error message should indicate pkgbuild operation: {error_msg}" ); } + + #[tokio::test] + /// What: Verify an oversized PKGBUILD response is rejected while reading. + /// + /// Inputs: + /// - A local response one byte above the approved 10 MiB ceiling. + /// + /// Output: + /// - `InputTooLong` retaining PKGBUILD-operation and package context. + /// + /// Details: + /// - The response is inert bytes and is never sourced or executed. + async fn oversized_aur_pkgbuild_response_is_rejected() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/PKGBUILD")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(vec![ + b'x'; + MAX_AUR_PKGBUILD_RESPONSE_BYTES + + 1 + ])) + .mount(&server) + .await; + + let error = + perform_pkgbuild_request(&Client::new(), &format!("{}/PKGBUILD", server.uri()), "yay") + .await + .expect_err("oversized PKGBUILD response must fail"); + let message = error.to_string(); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: MAX_AUR_PKGBUILD_RESPONSE_BYTES, + .. + } + )); + assert!(message.contains("PKGBUILD")); + assert!(message.contains("yay")); + } + + #[tokio::test] + /// What: Preserve PKGBUILD operation context for status and UTF-8 failures. + /// + /// Inputs: + /// - Local HTTP 404 and invalid UTF-8 responses for package `yay`. + /// + /// Output: + /// - Contextual errors identifying PKGBUILD and the package. + /// + /// Details: + /// - Neither response body is logged, interpreted, sourced, or executed. + async fn aur_pkgbuild_status_and_utf8_errors_are_contextual() { + for (path_value, template) in [ + ("/status", ResponseTemplate::new(404)), + ( + "/utf8", + ResponseTemplate::new(200).set_body_bytes([0xf0, 0x28, 0x8c]), + ), + ] { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(path_value)) + .respond_with(template) + .mount(&server) + .await; + + let error = perform_pkgbuild_request( + &Client::new(), + &format!("{}{path_value}", server.uri()), + "yay", + ) + .await + .expect_err("invalid PKGBUILD response must fail"); + let message = error.to_string(); + + assert!(message.contains("PKGBUILD")); + assert!(message.contains("yay")); + } + } + + #[tokio::test] + /// What: Return a normal bounded PKGBUILD fixture unchanged. + /// + /// Inputs: + /// - An inert local PKGBUILD string. + /// + /// Output: + /// - Exact UTF-8 content returned to the caller. + /// + /// Details: + /// - Fetching remains data-only and does not evaluate shell text. + async fn normal_aur_pkgbuild_fixture_is_read() { + let server = MockServer::start().await; + let pkgbuild = "pkgname=yay\npkgver=1\n"; + Mock::given(method("GET")) + .and(path("/PKGBUILD")) + .respond_with(ResponseTemplate::new(200).set_body_string(pkgbuild)) + .mount(&server) + .await; + + let body = + perform_pkgbuild_request(&Client::new(), &format!("{}/PKGBUILD", server.uri()), "yay") + .await + .expect("normal PKGBUILD fixture"); + + assert_eq!(body, pkgbuild); + } } diff --git a/src/deps/srcinfo.rs b/src/deps/srcinfo.rs index c3dbe50..e43ea7d 100644 --- a/src/deps/srcinfo.rs +++ b/src/deps/srcinfo.rs @@ -13,6 +13,10 @@ use crate::types::dependency::SrcinfoData; #[cfg(feature = "aur")] use crate::aur::utils::percent_encode; +/// Maximum accepted AUR `.SRCINFO` response body size in bytes. +#[cfg(feature = "aur")] +const MAX_AUR_SRCINFO_RESPONSE_BYTES: usize = 10 * 1024 * 1024; + /// What: Store one split-package output for graph-only `.SRCINFO` resolution. /// /// Inputs: @@ -469,48 +473,71 @@ pub fn parse_srcinfo(content: &str) -> SrcinfoData { /// - Requires the `aur` feature to be enabled. #[cfg(feature = "aur")] pub async fn fetch_srcinfo(client: &reqwest::Client, name: &str) -> Result { - use crate::error::ArchToolkitError; - let url = format!( "https://aur.archlinux.org/cgit/aur.git/plain/.SRCINFO?h={}", percent_encode(name) ); - tracing::debug!("Fetching .SRCINFO from: {}", url); + fetch_srcinfo_from_url(client, name, &url).await +} + +/// What: Fetch and validate one bounded `.SRCINFO` document from a selected URL. +/// +/// Inputs: +/// - `client`: Reqwest HTTP client retaining caller timeout and transport policy. +/// - `name`: AUR package name retained in every status, body, and parse error. +/// - `url`: Request URL selected by the public AUR endpoint wrapper or a local test. +/// +/// Output: +/// - Validated `.SRCINFO` text within [`MAX_AUR_SRCINFO_RESPONSE_BYTES`]. +/// +/// Details: +/// - Streams without executing, sourcing, expanding, or logging response content. +/// - The URL remains private to avoid logging a full untrusted value. +#[cfg(feature = "aur")] +async fn fetch_srcinfo_from_url(client: &reqwest::Client, name: &str, url: &str) -> Result { + use crate::error::ArchToolkitError; + tracing::debug!(package = %name, "fetching AUR .SRCINFO"); let response = client - .get(&url) + .get(url) .send() .await .map_err(ArchToolkitError::Network)?; - - if !response.status().is_success() { + let status = response.status(); + if !status.is_success() { return Err(ArchToolkitError::InvalidInput(format!( - "HTTP request failed with status: {}", - response.status() + "AUR .SRCINFO fetch failed for package '{name}' with status {status}" ))); } - let text = response.text().await.map_err(ArchToolkitError::Network)?; + let resource_label = format!("AUR .SRCINFO for package '{name}'"); + let text = crate::http::read_bounded_response_text( + response, + MAX_AUR_SRCINFO_RESPONSE_BYTES, + &resource_label, + |error| { + ArchToolkitError::Parse(format!( + "{resource_label} response body read failed: {error}" + )) + }, + ) + .await?; if text.trim().is_empty() { return Err(ArchToolkitError::EmptyInput { - field: "srcinfo_content".to_string(), - message: "Empty .SRCINFO content".to_string(), + field: format!("AUR .SRCINFO response for package '{name}'"), + message: "response body was empty".to_string(), }); } - - // Check if we got an HTML error page instead of .SRCINFO content if text.trim_start().starts_with(" Resulterror"), + ), + ] { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(path_value)) + .respond_with(template) + .mount(&server) + .await; + + let error = fetch_srcinfo_from_url( + &reqwest::Client::new(), + "yay", + &format!("{}{path_value}", server.uri()), + ) + .await + .expect_err("invalid .SRCINFO response must fail"); + let message = error.to_string(); + + assert!(message.contains(".SRCINFO")); + assert!(message.contains("yay")); + } + } + + #[cfg(feature = "aur")] + #[tokio::test] + /// What: Return a normal bounded `.SRCINFO` fixture unchanged. + /// + /// Inputs: + /// - A local valid metadata document for package `yay`. + /// + /// Output: + /// - Exact source text ready for caller-controlled parsing. + /// + /// Details: + /// - The fetch path validates markers but never executes metadata content. + async fn normal_aur_srcinfo_fixture_is_read() { + let server = MockServer::start().await; + let srcinfo = "pkgbase = yay\npkgname = yay\npkgver = 1\n"; + Mock::given(method("GET")) + .and(path("/.SRCINFO")) + .respond_with(ResponseTemplate::new(200).set_body_string(srcinfo)) + .mount(&server) + .await; + + let body = fetch_srcinfo_from_url( + &reqwest::Client::new(), + "yay", + &format!("{}/.SRCINFO", server.uri()), + ) + .await + .expect("normal .SRCINFO fixture"); + + assert_eq!(body, srcinfo); + } } diff --git a/src/http.rs b/src/http.rs new file mode 100644 index 0000000..fad9f25 --- /dev/null +++ b/src/http.rs @@ -0,0 +1,381 @@ +//! Shared private HTTP response-body safeguards. + +use crate::error::{ArchToolkitError, Result}; + +/// What: Read one HTTP response body with a strict streamed byte ceiling and UTF-8 validation. +/// +/// Inputs: +/// - `response`: Successful response whose body remains unread. +/// - `maximum_bytes`: Maximum accepted body length in bytes. +/// - `resource_label`: Operation and resource context used in errors. +/// - `map_read_error`: Maps streamed transport errors to the caller's existing error variant. +/// +/// Output: +/// - The complete UTF-8 body when it does not exceed `maximum_bytes`. +/// +/// Details: +/// - Rejects an oversized declared length before allocating or polling the body. +/// - Enforces the same ceiling on every streamed chunk when the length is absent or inaccurate. +/// - Stops as soon as the next chunk would cross the ceiling and never logs body content. +pub async fn read_bounded_response_text( + mut response: reqwest::Response, + maximum_bytes: usize, + resource_label: &str, + map_read_error: F, +) -> Result +where + F: FnOnce(reqwest::Error) -> ArchToolkitError, +{ + if let Some(length) = response.content_length() + && length > u64::try_from(maximum_bytes).unwrap_or(u64::MAX) + { + return Err(response_too_large( + resource_label, + maximum_bytes, + usize::try_from(length).unwrap_or(usize::MAX), + )); + } + + let initial_capacity = response + .content_length() + .and_then(|length| usize::try_from(length).ok()) + .unwrap_or(0); + let mut body = Vec::with_capacity(initial_capacity); + loop { + let chunk = match response.chunk().await { + Ok(Some(chunk)) => chunk, + Ok(None) => break, + Err(error) => return Err(map_read_error(error)), + }; + let observed_length = body.len().saturating_add(chunk.len()); + if observed_length > maximum_bytes { + return Err(response_too_large( + resource_label, + maximum_bytes, + observed_length, + )); + } + body.extend_from_slice(&chunk); + } + + String::from_utf8(body).map_err(|error| { + ArchToolkitError::Parse(format!( + "{resource_label} response body was not valid UTF-8: {error}" + )) + }) +} + +/// What: Build a contextual response-size error. +/// +/// Inputs: +/// - `resource_label`: Operation and package context. +/// - `maximum_bytes`: Configured byte ceiling. +/// - `actual_bytes`: Declared or observed body length. +/// +/// Output: +/// - `InputTooLong` using the existing public error surface. +/// +/// Details: +/// - The field identifies the response body rather than exposing an untrusted URL or content. +fn response_too_large( + resource_label: &str, + maximum_bytes: usize, + actual_bytes: usize, +) -> ArchToolkitError { + ArchToolkitError::InputTooLong { + field: format!("{resource_label} response body"), + max_length: maximum_bytes, + actual_length: actual_bytes, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Read, Write}; + use std::net::TcpListener; + use std::thread; + use std::time::Duration; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + /// What: Map a fixture transport error to a deterministic contextual crate error. + /// + /// Inputs: + /// - `error`: Reqwest body-stream error from a local fixture. + /// + /// Output: + /// - `Parse` retaining the fixture operation label. + /// + /// Details: + /// - Production callers instead retain their existing operation-specific network variants. + fn fixture_read_error(error: reqwest::Error) -> ArchToolkitError { + ArchToolkitError::Parse(format!("fixture body read failed: {}", error.without_url())) + } + + /// What: Start a one-response HTTP/1.1 fixture server. + /// + /// Inputs: + /// - `response`: Complete response bytes to write after one request. + /// - `linger`: Time to keep the connection open after flushing. + /// + /// Output: + /// - Local HTTP URL accepted by reqwest. + /// + /// Details: + /// - Supports connection-close and intentionally incomplete chunked fixtures unavailable in wiremock. + fn spawn_raw_response(response: Vec, linger: Duration) -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind raw HTTP fixture"); + let address = listener.local_addr().expect("raw HTTP fixture address"); + thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept fixture request"); + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .expect("set fixture read timeout"); + let mut request = [0_u8; 2048]; + let _ = stream.read(&mut request); + stream.write_all(&response).expect("write fixture response"); + stream.flush().expect("flush fixture response"); + thread::sleep(linger); + }); + format!("http://{address}/fixture") + } + + /// What: Build a raw connection-close response without `Content-Length`. + /// + /// Inputs: + /// - `body`: Response bytes. + /// + /// Output: + /// - Complete HTTP response bytes. + /// + /// Details: + /// - Closing the connection delimits the body and exercises the missing-header path. + fn response_without_length(body: &[u8]) -> Vec { + let mut response = b"HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n".to_vec(); + response.extend_from_slice(body); + response + } + + #[tokio::test] + /// What: Reject a declared response length above the configured ceiling. + /// + /// Inputs: + /// - A wiremock body of nine bytes and an eight-byte ceiling. + /// + /// Output: + /// - `InputTooLong` reporting the declared length. + /// + /// Details: + /// - Hyper supplies an accurate `Content-Length` for the full response body. + async fn declared_oversize_is_rejected() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/body")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(b"123456789")) + .mount(&server) + .await; + let response = reqwest::get(format!("{}/body", server.uri())) + .await + .expect("declared-length fixture response"); + + let error = read_bounded_response_text(response, 8, "declared fixture", fixture_read_error) + .await + .expect_err("declared oversize must fail"); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: 8, + actual_length: 9, + .. + } + )); + } + + #[tokio::test] + /// What: Reject a dishonest overreported length before waiting for its short body. + /// + /// Inputs: + /// - A response declaring nine bytes, sending one byte, and lingering with an eight-byte ceiling. + /// + /// Output: + /// - Immediate `InputTooLong` based on the declared length. + /// + /// Details: + /// - Proves an inaccurate header cannot force allocation or a body wait above the bound. + async fn dishonest_declared_oversize_is_rejected_early() { + let response = + b"HTTP/1.1 200 OK\r\nContent-Length: 9\r\nConnection: close\r\n\r\nx".to_vec(); + let url = spawn_raw_response(response, Duration::from_secs(1)); + let response = reqwest::get(url).await.expect("dishonest-length response"); + + let result = tokio::time::timeout( + Duration::from_millis(250), + read_bounded_response_text(response, 8, "dishonest fixture", fixture_read_error), + ) + .await + .expect("declared oversize should reject before body completion"); + + assert!(matches!( + result, + Err(ArchToolkitError::InputTooLong { + max_length: 8, + actual_length: 9, + .. + }) + )); + } + + #[tokio::test] + /// What: Enforce the streamed ceiling when `Content-Length` is absent. + /// + /// Inputs: + /// - A connection-close response of nine bytes and an eight-byte ceiling. + /// + /// Output: + /// - `InputTooLong` after observing the oversized bytes. + /// + /// Details: + /// - The body is delimited only by EOF, so the header cannot provide safety. + async fn missing_length_oversize_is_rejected() { + let url = spawn_raw_response( + response_without_length(b"123456789"), + Duration::from_millis(0), + ); + let response = reqwest::get(url).await.expect("missing-length response"); + + let error = read_bounded_response_text(response, 8, "missing fixture", fixture_read_error) + .await + .expect_err("missing-length oversize must fail"); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: 8, + actual_length: 9, + .. + } + )); + } + + #[tokio::test] + /// What: Enforce streamed bytes when a short declared length conflicts with chunked framing. + /// + /// Inputs: + /// - A response declaring one byte while chunked framing delivers nine bytes. + /// + /// Output: + /// - `InputTooLong` based on the streamed body rather than the dishonest short header. + /// + /// Details: + /// - Transfer framing controls the delivered chunks; the byte ceiling remains authoritative. + async fn dishonest_short_length_cannot_bypass_stream_limit() { + let response = b"HTTP/1.1 200 OK\r\nContent-Length: 1\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n9\r\n123456789\r\n0\r\n\r\n".to_vec(); + let url = spawn_raw_response(response, Duration::from_millis(0)); + let response = reqwest::get(url) + .await + .expect("dishonest short-length response"); + + let error = + read_bounded_response_text(response, 8, "dishonest short fixture", fixture_read_error) + .await + .expect_err("streamed overflow must override a dishonest short length"); + + assert!(matches!( + error, + ArchToolkitError::InputTooLong { + max_length: 8, + actual_length: 9, + .. + } + )); + } + + #[tokio::test] + /// What: Stop immediately when an incomplete chunked body crosses the limit. + /// + /// Inputs: + /// - A nine-byte first chunk, no terminating chunk, and an eight-byte ceiling. + /// + /// Output: + /// - `InputTooLong` before the server closes the incomplete response. + /// + /// Details: + /// - A timeout shorter than the fixture linger proves the reader does not poll another chunk. + async fn chunked_overflow_stops_immediately() { + let response = b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n9\r\n123456789\r\n".to_vec(); + let url = spawn_raw_response(response, Duration::from_secs(1)); + let response = reqwest::get(url).await.expect("chunked fixture response"); + + let result = tokio::time::timeout( + Duration::from_millis(250), + read_bounded_response_text(response, 8, "chunked fixture", fixture_read_error), + ) + .await + .expect("overflow should stop before the next chunk"); + + assert!(matches!( + result, + Err(ArchToolkitError::InputTooLong { + max_length: 8, + actual_length: 9, + .. + }) + )); + } + + #[tokio::test] + /// What: Accept a UTF-8 body exactly at the configured byte ceiling. + /// + /// Inputs: + /// - An eight-byte response and an eight-byte ceiling. + /// + /// Output: + /// - The unchanged response string. + /// + /// Details: + /// - Establishes inclusive boundary behavior. + async fn exact_limit_is_accepted() { + let url = spawn_raw_response( + response_without_length(b"12345678"), + Duration::from_millis(0), + ); + let response = reqwest::get(url).await.expect("exact-limit response"); + + let body = read_bounded_response_text(response, 8, "exact fixture", fixture_read_error) + .await + .expect("exact limit should succeed"); + + assert_eq!(body, "12345678"); + } + + #[tokio::test] + /// What: Reject invalid UTF-8 only after enforcing the response ceiling. + /// + /// Inputs: + /// - A bounded three-byte sequence that is not valid UTF-8. + /// + /// Output: + /// - A contextual `Parse` error. + /// + /// Details: + /// - The error exposes no response-body content. + async fn invalid_utf8_is_rejected() { + let url = spawn_raw_response( + response_without_length(&[0xf0, 0x28, 0x8c]), + Duration::from_millis(0), + ); + let response = reqwest::get(url).await.expect("invalid UTF-8 response"); + + let error = read_bounded_response_text(response, 8, "UTF-8 fixture", fixture_read_error) + .await + .expect_err("invalid UTF-8 must fail"); + let message = error.to_string(); + + assert!(matches!(error, ArchToolkitError::Parse(_))); + assert!(message.contains("UTF-8 fixture")); + assert!(message.contains("not valid UTF-8")); + assert!(!message.contains('�')); + } +} diff --git a/src/install/batch.rs b/src/install/batch.rs index cfeaf06..f8306db 100644 --- a/src/install/batch.rs +++ b/src/install/batch.rs @@ -39,7 +39,7 @@ impl InstallPlan { /// - `&self`: The planned commands, in execution order. /// /// Output: - /// - Shell string like `sudo pacman -S ... && paru -S --aur ...`; + /// - Shell string like `sudo pacman -S ... -- && paru -S --aur ... -- `; /// empty string for an empty plan. /// /// Details: @@ -68,7 +68,8 @@ impl InstallPlan { /// )?; /// assert_eq!( /// plan.to_shell_string(), - /// "pacman -S --needed --noconfirm ripgrep && paru -S --aur --needed --noconfirm yay-bin" + /// "pacman -S --needed --noconfirm -- ripgrep \ + /// && paru -S --aur --needed --noconfirm -- yay-bin" /// ); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` @@ -100,6 +101,8 @@ impl InstallPlan { /// Details: /// - Official packages are grouped into a single `pacman` invocation, AUR packages /// into a single helper invocation (from Pacsea's `build_batch_install_command`). +/// - Both grouped commands inherit strict name validation and the `--` operand +/// terminator from the underlying direct builders. /// - Empty `targets` produces an empty plan (no error), so callers can pass /// through selection results unchecked. /// - arch-toolkit never executes the plan; run the commands with @@ -129,8 +132,14 @@ impl InstallPlan { /// None::<&std::collections::HashSet>, /// )?; /// assert_eq!(plan.commands.len(), 2); -/// assert_eq!(plan.commands[0].to_shell_string(), "sudo pacman -S --needed --noconfirm ripgrep"); -/// assert_eq!(plan.commands[1].to_shell_string(), "paru -S --aur --needed --noconfirm yay-bin"); +/// assert_eq!( +/// plan.commands[0].to_shell_string(), +/// "sudo pacman -S --needed --noconfirm -- ripgrep" +/// ); +/// assert_eq!( +/// plan.commands[1].to_shell_string(), +/// "paru -S --aur --needed --noconfirm -- yay-bin" +/// ); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` pub fn build_batch_install( @@ -225,7 +234,7 @@ mod tests { /// - Two official targets with sudo privilege. /// /// Output: - /// - One command: `sudo pacman -S --needed --noconfirm a b`. + /// - One command: `sudo pacman -S --needed --noconfirm -- a b`. /// /// Details: /// - Grouping mirrors Pacsea's single-invocation batching. @@ -242,7 +251,11 @@ mod tests { assert_eq!(plan.commands.len(), 1); assert_eq!( plan.commands[0].to_shell_string(), - "sudo pacman -S --needed --noconfirm a b" + "sudo pacman -S --needed --noconfirm -- a b" + ); + assert_eq!( + plan.commands[0].args, + ["pacman", "-S", "--needed", "--noconfirm", "--", "a", "b"] ); assert_eq!(plan.official, ["a", "b"]); assert!(plan.aur.is_empty()); @@ -255,7 +268,7 @@ mod tests { /// - Two AUR targets with paru and sudo configured. /// /// Output: - /// - One command without sudo: `paru -S --aur --needed --noconfirm x y`. + /// - One command without sudo: `paru -S --aur --needed --noconfirm -- x y`. /// /// Details: /// - Helpers must never be wrapped in sudo; they escalate internally. @@ -272,10 +285,37 @@ mod tests { assert_eq!(plan.commands.len(), 1); assert_eq!( plan.commands[0].to_shell_string(), - "paru -S --aur --needed --noconfirm x y" + "paru -S --aur --needed --noconfirm -- x y" ); } + #[test] + /// What: Verify batch planning rejects option-like package names. + /// + /// Inputs: + /// - Official and AUR targets named `--help`, `-S`, and `.hidden`. + /// + /// Output: + /// - `InvalidPackageName` from both routing branches. + /// + /// Details: + /// - Batch planning must inherit the leading-byte rule (U5). + fn batch_rejects_option_like_names() { + for evil in ["--help", "-S", ".hidden"] { + for target in [official(evil), aur_pkg(evil)] { + let err = build_batch_install( + std::slice::from_ref(&target), + Some(AurHelper::Paru), + Some(PrivilegeTool::Sudo), + &InstallOptions::default(), + NO_INSTALLED, + ) + .expect_err("batch should reject option-like names"); + assert!(matches!(err, ArchToolkitError::InvalidPackageName { .. })); + } + } + } + #[test] /// What: Verify mixed batches order pacman before the AUR helper. /// @@ -334,11 +374,11 @@ mod tests { .expect("plan"); assert_eq!( plan.commands[0].to_shell_string(), - "pacman -S --noconfirm vim" + "pacman -S --noconfirm -- vim" ); assert_eq!( plan.commands[1].to_shell_string(), - "paru -S --aur --needed --noconfirm fresh-pkg" + "paru -S --aur --needed --noconfirm -- fresh-pkg" ); } diff --git a/src/install/command.rs b/src/install/command.rs index 71990c0..6d3eb72 100644 --- a/src/install/command.rs +++ b/src/install/command.rs @@ -15,6 +15,17 @@ use super::shell::validate_package_names; /// byte-identical output. pub const NO_AUR_HELPER_MESSAGE: &str = "No AUR helper (paru/yay) found."; +/// Exit status used by shell-fallback bodies when no AUR helper is installed. +/// +/// `127` is the POSIX convention for "command not found", so callers can +/// distinguish a missing helper from a failed package operation. +const NO_AUR_HELPER_STATUS: u8 = 127; + +/// POSIX option terminator placed between flags and package operands. +/// +/// Prevents pacman, paru, and yay from parsing any operand as an option. +const OPERAND_TERMINATOR: &str = "--"; + /// What: Build a pacman install command for official repository packages. /// /// Inputs: @@ -22,12 +33,15 @@ pub const NO_AUR_HELPER_MESSAGE: &str = "No AUR helper (paru/yay) found."; /// - `options`: Flag options (`needed`, `noconfirm`; `aur_only` is ignored). /// /// Output: -/// - `Ok(CommandSpec)` like `pacman -S --needed --noconfirm `. +/// - `Ok(CommandSpec)` like `pacman -S --needed --noconfirm -- `. /// /// Details: /// - Does NOT prefix a privilege tool; use [`with_privilege`] for that. /// - Omit `--needed` (set `options.needed = false`) for explicit reinstalls, /// mirroring Pacsea's reinstall path. +/// - A `--` operand terminator separates flags from package names so pacman can +/// never reinterpret an operand as an option (defense in depth on top of +/// name validation). /// /// # Errors /// @@ -41,7 +55,7 @@ pub const NO_AUR_HELPER_MESSAGE: &str = "No AUR helper (paru/yay) found."; /// use arch_toolkit::types::install::InstallOptions; /// /// let spec = build_pacman_install(&["ripgrep", "fd"], &InstallOptions::default())?; -/// assert_eq!(spec.to_shell_string(), "pacman -S --needed --noconfirm ripgrep fd"); +/// assert_eq!(spec.to_shell_string(), "pacman -S --needed --noconfirm -- ripgrep fd"); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` pub fn build_pacman_install>( @@ -57,7 +71,7 @@ pub fn build_pacman_install>( if options.noconfirm { args.push("--noconfirm".to_string()); } - args.extend(names.iter().map(|n| n.as_ref().to_string())); + push_operands(&mut args, names); Ok(CommandSpec { program: "pacman".to_string(), args, @@ -72,12 +86,14 @@ pub fn build_pacman_install>( /// - `options`: Flag options (`needed`, `noconfirm`, `aur_only`). /// /// Output: -/// - `Ok(CommandSpec)` like `paru -S --aur --needed --noconfirm `. +/// - `Ok(CommandSpec)` like `paru -S --aur --needed --noconfirm -- `. /// /// Details: /// - `--aur` (when `options.aur_only`) ensures helpers do not prefer a sync /// database (e.g., Chaotic-AUR) when the same name exists on the AUR — /// matching Pacsea's `aur_install_helper_flags`. +/// - A `--` operand terminator separates flags from package names; paru and yay +/// forward it to pacman-style operand parsing. /// - AUR helpers must NOT run under sudo; they invoke sudo themselves for the /// pacman step. Do not wrap the result in [`with_privilege`]. /// @@ -93,7 +109,7 @@ pub fn build_pacman_install>( /// use arch_toolkit::types::install::{AurHelper, InstallOptions}; /// /// let spec = build_aur_install(AurHelper::Paru, &["yay-bin"], &InstallOptions::default())?; -/// assert_eq!(spec.to_shell_string(), "paru -S --aur --needed --noconfirm yay-bin"); +/// assert_eq!(spec.to_shell_string(), "paru -S --aur --needed --noconfirm -- yay-bin"); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` pub fn build_aur_install>( @@ -113,7 +129,7 @@ pub fn build_aur_install>( if options.noconfirm { args.push("--noconfirm".to_string()); } - args.extend(names.iter().map(|n| n.as_ref().to_string())); + push_operands(&mut args, names); Ok(CommandSpec { program: helper.binary_name().to_string(), args, @@ -128,11 +144,12 @@ pub fn build_aur_install>( /// - `noconfirm`: Pass `--noconfirm` for non-interactive removal. /// /// Output: -/// - `Ok(CommandSpec)` like `pacman -Rns --noconfirm `. +/// - `Ok(CommandSpec)` like `pacman -Rns --noconfirm -- `. /// /// Details: /// - Does NOT prefix a privilege tool; use [`with_privilege`] for that. /// - Cascade semantics ported from Pacsea's `CascadeMode`. +/// - A `--` operand terminator separates flags from package names. /// /// # Errors /// @@ -146,7 +163,7 @@ pub fn build_aur_install>( /// use arch_toolkit::types::install::CascadeMode; /// /// let spec = build_remove_command(&["ripgrep"], CascadeMode::CascadeWithConfigs, true)?; -/// assert_eq!(spec.to_shell_string(), "pacman -Rns --noconfirm ripgrep"); +/// assert_eq!(spec.to_shell_string(), "pacman -Rns --noconfirm -- ripgrep"); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` pub fn build_remove_command>( @@ -160,13 +177,31 @@ pub fn build_remove_command>( if noconfirm { args.push("--noconfirm".to_string()); } - args.extend(names.iter().map(|n| n.as_ref().to_string())); + push_operands(&mut args, names); Ok(CommandSpec { program: "pacman".to_string(), args, }) } +/// What: Append the `--` operand terminator followed by validated package names. +/// +/// Inputs: +/// - `args`: Argument vector already containing every flag for the command. +/// - `names`: Validated package names to place after the terminator. +/// +/// Output: +/// - Side effect: `args` gains `--` and then one entry per package name. +/// +/// Details: +/// - Called only by builders that take package operands; operand-free commands +/// such as `-Syu`, `-Syyu`, and `-Sua` must never gain a terminator. +/// - Callers must validate names first; this helper performs no validation. +fn push_operands>(args: &mut Vec, names: &[S]) { + args.push(OPERAND_TERMINATOR.to_string()); + args.extend(names.iter().map(|n| n.as_ref().to_string())); +} + /// What: Build a full-system update command. /// /// Inputs: @@ -287,7 +322,11 @@ pub fn build_aur_update_command(helper: AurHelper, noconfirm: bool) -> CommandSp /// Pacsea's `aur_install_body`. Prefer this when the command runs in an /// external terminal whose environment may differ from the caller's. /// - Names pass the same strict validation as all builders, so interpolating -/// them into the shell string is safe without quoting. +/// them into the shell string is safe without quoting. A `--` operand +/// terminator is emitted before the names for defense in depth. +/// - When neither helper exists the body writes [`NO_AUR_HELPER_MESSAGE`] to +/// stderr and exits the subshell with status 127, so callers see a failure +/// instead of a successful no-op. /// /// # Errors /// @@ -303,6 +342,8 @@ pub fn build_aur_update_command(helper: AurHelper, noconfirm: bool) -> CommandSp /// let body = aur_install_shell_fallback(&["yay-bin"], &InstallOptions::default())?; /// assert!(body.contains("if command -v paru >/dev/null 2>&1; then paru")); /// assert!(body.contains("elif command -v yay >/dev/null 2>&1; then yay")); +/// assert!(body.contains("--noconfirm -- yay-bin")); +/// assert!(body.contains("exit 127")); /// # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) /// ``` pub fn aur_install_shell_fallback>( @@ -326,7 +367,9 @@ pub fn aur_install_shell_fallback>( .map(std::convert::AsRef::as_ref) .collect::>() .join(" "); - Ok(helper_fallback_body(&format!("{flags} {joined}"))) + Ok(helper_fallback_body(&format!( + "{flags} {OPERAND_TERMINATOR} {joined}" + ))) } /// What: Build a shell body that updates AUR packages with runtime helper fallback. @@ -341,6 +384,8 @@ pub fn aur_install_shell_fallback>( /// Details: /// - Shell-time counterpart of [`build_aur_update_command`], for callers that /// spawn the update in an external terminal (Pacsea's system-update flow). +/// - Takes no package operands, so no `--` terminator is emitted. +/// - The no-helper branch writes to stderr and exits the subshell with 127. /// /// # Example /// @@ -366,12 +411,18 @@ pub fn aur_update_shell_fallback(noconfirm: bool) -> String { /// - `tail`: Flags and package names appended to the chosen helper. /// /// Output: -/// - Parenthesized `if/elif/else` snippet matching Pacsea's `aur_install_body`. +/// - Parenthesized `if/elif/else` snippet matching Pacsea's `aur_install_body`, +/// with a failing no-helper branch. +/// +/// Details: +/// - The `else` branch writes [`NO_AUR_HELPER_MESSAGE`] to stderr and runs +/// `exit 127`. Because the body is wrapped in `( ... )`, only the fallback +/// subshell terminates; the caller observes a non-zero status. fn helper_fallback_body(tail: &str) -> String { format!( "(if command -v paru >/dev/null 2>&1; then paru {tail}; \ elif command -v yay >/dev/null 2>&1; then yay {tail}; \ - else echo '{NO_AUR_HELPER_MESSAGE}'; fi)" + else echo '{NO_AUR_HELPER_MESSAGE}' >&2; exit {NO_AUR_HELPER_STATUS}; fi)" ) } @@ -451,7 +502,7 @@ mod tests { build_pacman_install(&["ripgrep"], &InstallOptions::default()).expect("build fresh"); assert_eq!( fresh.to_shell_string(), - "pacman -S --needed --noconfirm ripgrep" + "pacman -S --needed --noconfirm -- ripgrep" ); let reinstall_opts = InstallOptions { @@ -460,14 +511,18 @@ mod tests { }; let reinstall = build_pacman_install(&["ripgrep"], &reinstall_opts).expect("build reinstall"); - assert_eq!(reinstall.to_shell_string(), "pacman -S --noconfirm ripgrep"); + assert_eq!( + reinstall.to_shell_string(), + "pacman -S --noconfirm -- ripgrep" + ); let interactive = InstallOptions { noconfirm: false, ..Default::default() }; let spec = build_pacman_install(&["a", "b"], &interactive).expect("build interactive"); - assert_eq!(spec.to_shell_string(), "pacman -S --needed a b"); + assert_eq!(spec.to_shell_string(), "pacman -S --needed -- a b"); + assert_eq!(spec.args, ["-S", "--needed", "--", "a", "b"]); } #[test] @@ -486,7 +541,7 @@ mod tests { .expect("build"); assert_eq!( spec.to_shell_string(), - "paru -S --aur --needed --noconfirm yay-bin" + "paru -S --aur --needed --noconfirm -- yay-bin" ); let reinstall = InstallOptions { @@ -494,14 +549,17 @@ mod tests { ..Default::default() }; let spec2 = build_aur_install(AurHelper::Yay, &["yay-bin"], &reinstall).expect("build"); - assert_eq!(spec2.to_shell_string(), "yay -S --aur --noconfirm yay-bin"); + assert_eq!( + spec2.to_shell_string(), + "yay -S --aur --noconfirm -- yay-bin" + ); let no_aur_flag = InstallOptions { aur_only: false, ..Default::default() }; let spec3 = build_aur_install(AurHelper::Paru, &["x"], &no_aur_flag).expect("build"); - assert_eq!(spec3.to_shell_string(), "paru -S --needed --noconfirm x"); + assert_eq!(spec3.to_shell_string(), "paru -S --needed --noconfirm -- x"); } #[test] @@ -517,15 +575,16 @@ mod tests { /// - Matches Pacsea's `CascadeMode::flag()` semantics. fn remove_cascade_modes() { let basic = build_remove_command(&["pkg"], CascadeMode::Basic, false).expect("build basic"); - assert_eq!(basic.to_shell_string(), "pacman -R pkg"); + assert_eq!(basic.to_shell_string(), "pacman -R -- pkg"); let cascade = build_remove_command(&["pkg"], CascadeMode::Cascade, true).expect("build cascade"); - assert_eq!(cascade.to_shell_string(), "pacman -Rs --noconfirm pkg"); + assert_eq!(cascade.to_shell_string(), "pacman -Rs --noconfirm -- pkg"); let full = build_remove_command(&["a", "b"], CascadeMode::CascadeWithConfigs, true) .expect("build full"); - assert_eq!(full.to_shell_string(), "pacman -Rns --noconfirm a b"); + assert_eq!(full.to_shell_string(), "pacman -Rns --noconfirm -- a b"); + assert_eq!(full.args, ["-Rns", "--noconfirm", "--", "a", "b"]); } #[test] @@ -539,11 +598,11 @@ mod tests { /// /// Details: /// - Helper variant updates both official and AUR packages. + /// - Operand-free update commands must not gain a `--` terminator. fn update_commands() { - assert_eq!( - build_update_command(None, true).to_shell_string(), - "pacman -Syu --noconfirm" - ); + let pacman = build_update_command(None, true); + assert!(!pacman.args.iter().any(|arg| arg == "--")); + assert_eq!(pacman.to_shell_string(), "pacman -Syu --noconfirm"); assert_eq!( build_update_command(Some(AurHelper::Paru), false).to_shell_string(), "paru -Syu" @@ -587,7 +646,8 @@ mod tests { /// - Install and update fallback bodies with default options. /// /// Output: - /// - Parenthesized paru → yay `if/elif/else` with the exact error message. + /// - Parenthesized paru → yay `if/elif/else` with the exact error message, + /// an operand terminator, and a failing no-helper branch. /// /// Details: /// - Helper selection happens at shell execution time, not plan time. @@ -596,14 +656,18 @@ mod tests { .expect("build body"); assert_eq!( body, - "(if command -v paru >/dev/null 2>&1; then paru -S --aur --needed --noconfirm yay-bin; \ - elif command -v yay >/dev/null 2>&1; then yay -S --aur --needed --noconfirm yay-bin; \ - else echo 'No AUR helper (paru/yay) found.'; fi)" + "(if command -v paru >/dev/null 2>&1; \ + then paru -S --aur --needed --noconfirm -- yay-bin; \ + elif command -v yay >/dev/null 2>&1; \ + then yay -S --aur --needed --noconfirm -- yay-bin; \ + else echo 'No AUR helper (paru/yay) found.' >&2; exit 127; fi)" ); let update = aur_update_shell_fallback(false); assert!(update.contains("paru -Sua;")); + assert!(!update.contains("-Sua --")); assert!(update.contains(NO_AUR_HELPER_MESSAGE)); + assert!(update.contains("' >&2; exit 127; fi)")); let inj = aur_install_shell_fallback(&["bad;rm -rf /"], &InstallOptions::default()); assert!(matches!( @@ -634,12 +698,16 @@ mod tests { let sudo = with_privilege(PrivilegeTool::Sudo, spec.clone()); assert_eq!( sudo.to_shell_string(), - "sudo pacman -S --needed --noconfirm vim" + "sudo pacman -S --needed --noconfirm -- vim" + ); + assert_eq!( + sudo.args, + ["pacman", "-S", "--needed", "--noconfirm", "--", "vim"] ); let doas = with_privilege(PrivilegeTool::Doas, spec); assert_eq!( doas.to_shell_string(), - "doas pacman -S --needed --noconfirm vim" + "doas pacman -S --needed --noconfirm -- vim" ); } @@ -654,7 +722,27 @@ mod tests { /// /// Details: /// - Defense-in-depth: names are validated before any command is produced. + /// - Leading `-`/`.` names are rejected on every builder (U5). fn validation_errors() { + for evil in ["--help", "-S", ".hidden"] { + assert!( + build_pacman_install(&[evil], &InstallOptions::default()).is_err(), + "pacman install should reject {evil}" + ); + assert!( + build_aur_install(AurHelper::Paru, &[evil], &InstallOptions::default()).is_err(), + "AUR install should reject {evil}" + ); + assert!( + build_remove_command(&[evil], CascadeMode::Basic, true).is_err(), + "remove should reject {evil}" + ); + assert!( + aur_install_shell_fallback(&[evil], &InstallOptions::default()).is_err(), + "shell fallback should reject {evil}" + ); + } + let inj = build_pacman_install(&["good", "bad;rm -rf /"], &InstallOptions::default()); assert!(matches!( inj, diff --git a/src/install/mod.rs b/src/install/mod.rs index 6dc1c16..4d3797e 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -22,7 +22,7 @@ //! //! ```toml //! [dependencies] -//! arch-toolkit = { version = "0.2", features = ["install"] } +//! arch-toolkit = { version = "0.3", features = ["install"] } //! ``` //! //! # Examples @@ -73,7 +73,7 @@ //! PrivilegeTool::Sudo, //! build_remove_command(&["old-package"], CascadeMode::CascadeWithConfigs, true)?, //! ); -//! assert_eq!(spec.to_shell_string(), "sudo pacman -Rns --noconfirm old-package"); +//! assert_eq!(spec.to_shell_string(), "sudo pacman -Rns --noconfirm -- old-package"); //! # Ok::<(), arch_toolkit::error::ArchToolkitError>(()) //! ``` //! diff --git a/src/install/shell.rs b/src/install/shell.rs index d84d7c4..4fb4459 100644 --- a/src/install/shell.rs +++ b/src/install/shell.rs @@ -48,12 +48,17 @@ pub fn shell_single_quote(s: &str) -> String { /// - `name`: Candidate package name to validate. /// /// Output: -/// - `true` when `name` is non-empty and every byte is one of `a-z`, `0-9`, -/// `@`, `.`, `_`, `+`, `-`. +/// - `true` when `name` starts with a lowercase ASCII letter or digit and every +/// remaining byte is one of `a-z`, `0-9`, `@`, `.`, `_`, `+`, `-`. /// /// Details: /// - Defense-in-depth gate before command construction, matching Arch's /// package naming rules (lowercase only). +/// - The first byte may not be `-` or `.`, so a name can never be parsed as an +/// option (`--help`, `-S`) or a hidden path, even before the `--` operand +/// terminator that all builders emit. +/// - Internal `@ . _ + -` remain valid, preserving `lib32-*`, split packages, +/// versioned names such as `python3.12`, and `+` names. /// - Ported from Pacsea's `install/utils.rs`. /// /// # Example @@ -63,18 +68,27 @@ pub fn shell_single_quote(s: &str) -> String { /// /// assert!(is_safe_package_name("ripgrep")); /// assert!(is_safe_package_name("libc++")); +/// assert!(is_safe_package_name("lib32-glibc")); /// assert!(!is_safe_package_name("bad;rm -rf")); /// assert!(!is_safe_package_name("Upper")); +/// assert!(!is_safe_package_name("--help")); +/// assert!(!is_safe_package_name(".hidden")); /// assert!(!is_safe_package_name("")); /// ``` #[must_use] pub fn is_safe_package_name(name: &str) -> bool { - !name.is_empty() - && name.bytes().all(|byte| { - byte.is_ascii_lowercase() - || byte.is_ascii_digit() - || matches!(byte, b'@' | b'.' | b'_' | b'+' | b'-') - }) + let mut bytes = name.bytes(); + let Some(first) = bytes.next() else { + return false; + }; + if !first.is_ascii_lowercase() && !first.is_ascii_digit() { + return false; + } + bytes.all(|byte| { + byte.is_ascii_lowercase() + || byte.is_ascii_digit() + || matches!(byte, b'@' | b'.' | b'_' | b'+' | b'-') + }) } /// What: Validate a list of package names against the strict install-command allowlist. @@ -89,6 +103,8 @@ pub fn is_safe_package_name(name: &str) -> bool { /// /// Details: /// - Centralises validation so all install builders apply the same safety policy. +/// - The reported pattern states the leading-byte restriction that prevents +/// option confusion. /// - Ported from Pacsea's `install/utils.rs`, adapted to `ArchToolkitError`. /// /// # Errors @@ -102,7 +118,9 @@ pub fn validate_package_names>(names: &[S], context: &str) -> Resu { return Err(ArchToolkitError::InvalidPackageName { name: invalid.as_ref().to_string(), - reason: format!("invalid name for {context}; allowed pattern: ^[a-z0-9@._+-]+$"), + reason: format!( + "invalid name for {context}; allowed pattern: ^[a-z0-9][a-z0-9@._+-]*$" + ), }); } Ok(()) @@ -214,7 +232,8 @@ mod tests { /// - `true` only for names matching `^[a-z0-9@._+-]+$`. /// /// Details: - /// - Uppercase, whitespace, and shell metacharacters must be rejected. + /// - Uppercase, whitespace, shell metacharacters, and leading `-`/`.` must + /// be rejected; internal punctuation must stay valid. fn safe_names() { for good in [ "ripgrep", @@ -222,10 +241,27 @@ mod tests { "lib32-glibc", "python3.12", "a@b_c", + "0ad", ] { assert!(is_safe_package_name(good), "{good} should be valid"); } - for bad in ["", "Upper", "a b", "x;y", "$(rm)", "a`b`", "name'quote"] { + for bad in [ + "", + "Upper", + "a b", + "x;y", + "$(rm)", + "a`b`", + "name'quote", + "-S", + "--help", + "-", + ".hidden", + ".", + "@scoped", + "_leading", + "+plus", + ] { assert!(!is_safe_package_name(bad), "{bad} should be invalid"); } } @@ -243,12 +279,15 @@ mod tests { /// - Valid lists must pass unchanged. fn validation() { assert!(validate_package_names(&["vim", "git"], "test").is_ok()); + let leading = validate_package_names(&["vim", "--help"], "test install"); + assert!(leading.is_err(), "leading option names must be rejected"); let err = validate_package_names(&["vim", "bad;name"], "test install") .expect_err("should reject"); match err { crate::error::ArchToolkitError::InvalidPackageName { name, reason } => { assert_eq!(name, "bad;name"); assert!(reason.contains("test install")); + assert!(reason.contains("^[a-z0-9][a-z0-9@._+-]*$")); } other => panic!("unexpected error: {other:?}"), } diff --git a/src/lib.rs b/src/lib.rs index 92505d6..c42038a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,6 +155,9 @@ pub mod health; #[cfg(feature = "aur")] mod env; +#[cfg(feature = "aur")] +mod http; + #[cfg(feature = "deps")] pub mod deps; diff --git a/tests/compatibility_contract.rs b/tests/compatibility_contract.rs index 7d8d5b8..4794e18 100644 --- a/tests/compatibility_contract.rs +++ b/tests/compatibility_contract.rs @@ -4,7 +4,9 @@ feature = "aur", feature = "deps", feature = "index", - feature = "install" + feature = "install", + feature = "news", + feature = "sandbox" ))] mod tests { #[cfg(all(any(feature = "install", feature = "deps"), unix))] @@ -129,7 +131,7 @@ mod tests { .expect("build command plan"); assert_eq!( spec.to_shell_string(), - "pacman -S --needed --noconfirm ripgrep" + "pacman -S --needed --noconfirm -- ripgrep" ); assert!(!marker.exists(), "building a command must never execute it"); } @@ -172,4 +174,134 @@ mod tests { .is_empty() ); } + + #[cfg(feature = "aur")] + #[test] + /// What: Freeze the AUR model fields consumed by Pacsea. + /// + /// Inputs: + /// - Representative search, detail, and comment values. + /// + /// Output: + /// - Direct field access and serialization retain the consumer-facing data. + /// + /// Details: + /// - This is an aggregate compatibility fixture, not a network test. + fn pacsea_aur_model_contract() { + use arch_toolkit::types::package::{AurComment, AurPackage, AurPackageDetails}; + + let package = AurPackage { + name: "paru".to_string(), + version: "2.1.0-1".to_string(), + description: "AUR helper".to_string(), + popularity: Some(9.5), + out_of_date: Some(1_700_000_000), + orphaned: false, + maintainer: Some("maintainer".to_string()), + }; + assert_eq!(package.name, "paru"); + assert_eq!(package.maintainer.as_deref(), Some("maintainer")); + + let details = AurPackageDetails { + name: "paru".to_string(), + version: "2.1.0-1".to_string(), + depends: vec!["pacman".to_string()], + make_depends: vec!["cargo".to_string()], + opt_depends: vec!["bat: colored output".to_string()], + provides: vec!["aur-helper".to_string()], + conflicts: vec!["paru-bin".to_string()], + num_votes: Some(42), + ..Default::default() + }; + assert_eq!(details.depends, ["pacman"]); + assert_eq!(details.provides, ["aur-helper"]); + assert_eq!(details.num_votes, Some(42)); + + let comment = AurComment { + id: Some("123".to_string()), + author: "alice".to_string(), + date: "2026-08-08".to_string(), + date_timestamp: Some(1_786_147_200), + date_url: Some("https://aur.archlinux.org/packages/paru#comment-123".to_string()), + content: "Pinned guidance".to_string(), + pinned: true, + }; + let serialized = serde_json::to_value(comment).expect("serialize AUR comment"); + assert_eq!(serialized["id"], "123"); + assert_eq!(serialized["pinned"], true); + } + + #[cfg(feature = "news")] + #[test] + /// What: Freeze advisory identity, severity, package extraction, and serde fields for Pacsea. + /// + /// Inputs: + /// - A representative security advisory. + /// + /// Output: + /// - Stable ID, rank, package list, and serialized representation. + /// + /// Details: + /// - Caller read-state remains outside the library; this fixture covers only shared data. + fn pacsea_news_model_contract() { + use arch_toolkit::types::news::{AdvisorySeverity, SecurityAdvisory}; + + let advisory = SecurityAdvisory { + id: "ASA-202608-1".to_string(), + date: "2026-08-08".to_string(), + title: "ASA-202608-1: openssl: multiple issues".to_string(), + summary: Some("Multiple issues".to_string()), + url: Some("https://security.archlinux.org/ASA-202608-1".to_string()), + severity: AdvisorySeverity::High, + packages: vec!["openssl".to_string()], + }; + assert_eq!(advisory.id, "ASA-202608-1"); + assert_eq!(advisory.severity.rank(), 4); + assert_eq!(advisory.packages, ["openssl"]); + let serialized = serde_json::to_value(advisory).expect("serialize advisory"); + assert_eq!(serialized["severity"], "High"); + } + + #[cfg(feature = "sandbox")] + #[test] + /// What: Freeze sandbox dependency-delta serde and version-state behavior for Pacsea. + /// + /// Inputs: + /// - One installed but version-unsatisfied dependency and one missing dependency. + /// + /// Output: + /// - Stable roundtrip data, missing-package list, and readiness behavior. + /// + /// Details: + /// - Pacsea remains responsible for combining installation and version satisfaction in its adapter. + fn pacsea_sandbox_model_contract() { + use arch_toolkit::types::sandbox::{DependencyDelta, SandboxInfo}; + + let info = SandboxInfo { + package_name: "demo".to_string(), + depends: vec![ + DependencyDelta { + name: "openssl>=3.5".to_string(), + is_installed: true, + installed_version: Some("3.4".to_string()), + version_satisfied: false, + }, + DependencyDelta { + name: "missing-runtime".to_string(), + is_installed: false, + installed_version: None, + version_satisfied: false, + }, + ], + ..Default::default() + }; + assert_eq!(info.missing_packages(), ["missing-runtime"]); + assert!(!info.is_ready_to_build()); + assert!(!info.depends[0].version_satisfied); + + let serialized = serde_json::to_string(&info).expect("serialize sandbox info"); + let roundtrip: SandboxInfo = + serde_json::from_str(&serialized).expect("deserialize sandbox info"); + assert_eq!(roundtrip, info); + } } diff --git a/tests/install_integration.rs b/tests/install_integration.rs index 8c0f6e2..257ffb1 100644 --- a/tests/install_integration.rs +++ b/tests/install_integration.rs @@ -5,14 +5,79 @@ mod tests { use std::collections::HashSet; use arch_toolkit::install::{ - build_batch_install, build_pacman_install, build_remove_command, build_update_command, - with_privilege, + NO_AUR_HELPER_MESSAGE, aur_install_shell_fallback, aur_update_shell_fallback, + build_aur_install, build_aur_update_command, build_batch_install, + build_force_sync_update_command, build_pacman_install, build_remove_command, + build_update_command, is_safe_package_name, with_privilege, }; use arch_toolkit::types::install::{ AurHelper, CascadeMode, CommandSpec, InstallOptions, PrivilegeTool, }; use arch_toolkit::{PackageRef, PackageSource}; + /// What: Collect the argv of a `CommandSpec` as owned strings. + /// + /// Inputs: + /// - `spec`: The command specification to inspect. + /// + /// Output: + /// - Vector containing the program followed by every argument. + /// + /// Details: + /// - Used to assert exact argv including the `--` operand terminator, + /// independently of shell rendering. + fn argv(spec: &CommandSpec) -> Vec { + let mut out = vec![spec.program.clone()]; + out.extend(spec.args.iter().cloned()); + out + } + + /// What: Build a `PackageRef` for an AUR target with a fixed version. + /// + /// Inputs: + /// - `name`: Package name to route to the AUR helper. + /// + /// Output: + /// - `PackageRef` whose source is `PackageSource::Aur`. + /// + /// Details: + /// - Keeps batch-planning regression tests short and deterministic. + fn aur_target(name: &str) -> PackageRef { + PackageRef { + name: name.to_string(), + version: "1.0.0".to_string(), + source: PackageSource::Aur, + } + } + + /// What: Build a `PackageRef` for an official target with fixed metadata. + /// + /// Inputs: + /// - `name`: Package name to route to pacman. + /// + /// Output: + /// - `PackageRef` whose source is `PackageSource::Official`. + /// + /// Details: + /// - Keeps batch-planning regression tests short and deterministic. + fn official_target(name: &str) -> PackageRef { + PackageRef::official(name, "1.0.0", "extra", "x86_64") + } + + /// Names that must never reach a built command because their first byte can + /// be parsed as an option or a hidden-path prefix. + const OPTION_LIKE_NAMES: [&str; 7] = ["--help", "-S", "-Rns", "--", "-", ".hidden", "."]; + + /// Valid Arch names that keep internal `@ . _ + -` punctuation and must stay accepted. + const VALID_NAMES: [&str; 6] = [ + "ripgrep", + "lib32-glibc", + "python3.12", + "gcc12+libs", + "a@b_c", + "0ad", + ]; + #[test] /// What: Verify the full workflow: plan a mixed batch, wrap, and render. /// @@ -47,12 +112,12 @@ mod tests { // fd is installed → the official group drops --needed (reinstall path) assert_eq!( plan.commands[0].to_shell_string(), - "sudo pacman -S --noconfirm ripgrep fd" + "sudo pacman -S --noconfirm -- ripgrep fd" ); // AUR group has no reinstalls → keeps --needed, never sudo-wrapped assert_eq!( plan.commands[1].to_shell_string(), - "paru -S --aur --needed --noconfirm paru-bin" + "paru -S --aur --needed --noconfirm -- paru-bin" ); } @@ -75,7 +140,7 @@ mod tests { let cmd = spec.to_command(); assert_eq!(cmd.get_program(), "doas"); let args: Vec<_> = cmd.get_args().map(|a| a.to_string_lossy()).collect(); - assert_eq!(args, ["pacman", "-Rs", "--noconfirm", "old-pkg"]); + assert_eq!(args, ["pacman", "-Rs", "--noconfirm", "--", "old-pkg"]); } #[test] @@ -141,6 +206,394 @@ mod tests { ); } + #[test] + /// What: Verify option-like package names are rejected by every install path. + /// + /// Inputs: + /// - Names starting with `-` or `.` such as `--help`, `-S`, and `.hidden`. + /// + /// Output: + /// - `is_safe_package_name` is `false` and every builder returns an error. + /// + /// Details: + /// - Covers direct pacman/AUR install, remove, batch planning (official and + /// AUR routes), and the runtime shell fallback (U5). + fn option_like_names_rejected_across_all_builders() { + for evil in OPTION_LIKE_NAMES { + assert!( + !is_safe_package_name(evil), + "{evil} must not be a safe package name" + ); + assert!( + build_pacman_install(&[evil], &InstallOptions::default()).is_err(), + "pacman install should reject {evil}" + ); + assert!( + build_aur_install(AurHelper::Paru, &[evil], &InstallOptions::default()).is_err(), + "AUR install should reject {evil}" + ); + assert!( + build_remove_command(&[evil], CascadeMode::CascadeWithConfigs, true).is_err(), + "remove should reject {evil}" + ); + assert!( + aur_install_shell_fallback(&[evil], &InstallOptions::default()).is_err(), + "shell fallback should reject {evil}" + ); + for target in [official_target(evil), aur_target(evil)] { + assert!( + build_batch_install( + std::slice::from_ref(&target), + Some(AurHelper::Paru), + Some(PrivilegeTool::Sudo), + &InstallOptions::default(), + None::<&HashSet>, + ) + .is_err(), + "batch should reject {evil}" + ); + } + } + } + + #[test] + /// What: Verify legitimate Arch names with internal punctuation stay accepted. + /// + /// Inputs: + /// - `lib32-glibc`, `python3.12`, `gcc12+libs`, `a@b_c`, `0ad`, `ripgrep`. + /// + /// Output: + /// - Every direct, remove, batch, and fallback builder succeeds. + /// + /// Details: + /// - Guards the tightened first-byte rule against false rejections (U5). + fn valid_names_with_internal_punctuation_still_accepted() { + for good in VALID_NAMES { + assert!(is_safe_package_name(good), "{good} must remain valid"); + assert!( + build_pacman_install(&[good], &InstallOptions::default()).is_ok(), + "pacman install should accept {good}" + ); + assert!( + build_aur_install(AurHelper::Yay, &[good], &InstallOptions::default()).is_ok(), + "AUR install should accept {good}" + ); + assert!( + build_remove_command(&[good], CascadeMode::Basic, false).is_ok(), + "remove should accept {good}" + ); + assert!( + aur_install_shell_fallback(&[good], &InstallOptions::default()).is_ok(), + "shell fallback should accept {good}" + ); + let target = official_target(good); + assert!( + build_batch_install( + std::slice::from_ref(&target), + None, + None, + &InstallOptions::default(), + None::<&HashSet>, + ) + .is_ok(), + "batch should accept {good}" + ); + } + } + + #[test] + /// What: Verify `--` separates flags from operands in direct install/remove argv. + /// + /// Inputs: + /// - pacman install, AUR helper install, and remove specifications. + /// + /// Output: + /// - Exact argv with `--` after the last flag and before the first name. + /// + /// Details: + /// - Also asserts privilege wrapping keeps the terminator in place and does + /// not reinterpret it (U6). + fn direct_commands_place_operand_terminator_before_names() { + let install = + build_pacman_install(&["ripgrep", "fd"], &InstallOptions::default()).expect("build"); + assert_eq!( + argv(&install), + [ + "pacman", + "-S", + "--needed", + "--noconfirm", + "--", + "ripgrep", + "fd" + ] + ); + + let aur = build_aur_install(AurHelper::Paru, &["yay-bin"], &InstallOptions::default()) + .expect("build"); + assert_eq!( + argv(&aur), + [ + "paru", + "-S", + "--aur", + "--needed", + "--noconfirm", + "--", + "yay-bin" + ] + ); + + let remove = build_remove_command(&["old-pkg"], CascadeMode::CascadeWithConfigs, true) + .expect("build"); + assert_eq!( + argv(&remove), + ["pacman", "-Rns", "--noconfirm", "--", "old-pkg"] + ); + + let privileged = with_privilege(PrivilegeTool::Sudo, remove); + assert_eq!( + argv(&privileged), + ["sudo", "pacman", "-Rns", "--noconfirm", "--", "old-pkg"] + ); + assert_eq!( + privileged.to_shell_string(), + "sudo pacman -Rns --noconfirm -- old-pkg" + ); + } + + #[test] + /// What: Verify batch plans terminate operands and preserve ordering and `&&`. + /// + /// Inputs: + /// - Mixed official/AUR targets with sudo and paru. + /// + /// Output: + /// - Privileged pacman command first, unprivileged helper second, both with + /// `--`, joined by `&&`. + /// + /// Details: + /// - Guards U6 for the batch path and the short-circuit rendering contract. + fn batch_plan_terminates_operands_and_preserves_order() { + let targets = vec![aur_target("paru-bin"), official_target("ripgrep")]; + let plan = build_batch_install( + &targets, + Some(AurHelper::Paru), + Some(PrivilegeTool::Sudo), + &InstallOptions::default(), + None::<&HashSet>, + ) + .expect("plan should build"); + + assert_eq!( + argv(&plan.commands[0]), + [ + "sudo", + "pacman", + "-S", + "--needed", + "--noconfirm", + "--", + "ripgrep" + ] + ); + assert_eq!( + argv(&plan.commands[1]), + [ + "paru", + "-S", + "--aur", + "--needed", + "--noconfirm", + "--", + "paru-bin" + ] + ); + assert_eq!( + plan.to_shell_string(), + "sudo pacman -S --needed --noconfirm -- ripgrep && \ + paru -S --aur --needed --noconfirm -- paru-bin" + ); + } + + #[test] + /// What: Verify update commands gain no artificial operand terminator. + /// + /// Inputs: + /// - `-Syu`, `-Syyu`, and `-Sua` builders for pacman and helpers. + /// + /// Output: + /// - Argv without `--` because these commands take no package operands. + /// + /// Details: + /// - Prevents the U6 change from altering operand-free update commands. + fn update_commands_have_no_operand_terminator() { + for spec in [ + build_update_command(None, true), + build_update_command(Some(AurHelper::Paru), false), + build_force_sync_update_command(None, true), + build_aur_update_command(AurHelper::Yay, true), + ] { + assert!( + !spec.args.iter().any(|arg| arg == "--"), + "update command must not contain an operand terminator: {}", + spec.to_shell_string() + ); + } + assert_eq!( + aur_update_shell_fallback(true), + "(if command -v paru >/dev/null 2>&1; then paru -Sua --noconfirm; \ + elif command -v yay >/dev/null 2>&1; then yay -Sua --noconfirm; \ + else echo 'No AUR helper (paru/yay) found.' >&2; exit 127; fi)" + ); + } + + #[test] + /// What: Verify the install fallback body golden with terminator and failure branch. + /// + /// Inputs: + /// - Default install options for a single AUR package. + /// + /// Output: + /// - Exact shell body with `--` before the operand and a stderr/exit-127 + /// no-helper branch. + /// + /// Details: + /// - Message text must stay byte-identical for migrating callers (U6, U7). + fn install_fallback_body_golden() { + let body = aur_install_shell_fallback(&["yay-bin"], &InstallOptions::default()) + .expect("build body"); + assert_eq!( + body, + "(if command -v paru >/dev/null 2>&1; then paru -S --aur --needed --noconfirm -- yay-bin; \ + elif command -v yay >/dev/null 2>&1; then yay -S --aur --needed --noconfirm -- yay-bin; \ + else echo 'No AUR helper (paru/yay) found.' >&2; exit 127; fi)" + ); + assert!(body.contains(NO_AUR_HELPER_MESSAGE)); + } + + #[cfg(unix)] + #[test] + /// What: Verify fallback execution prefers paru, falls back to yay, and fails loudly. + /// + /// Inputs: + /// - Isolated `PATH` fixtures containing paru only, yay only, or no helper. + /// + /// Output: + /// - Recorded helper argv including `--`, and status 127 with the message on + /// stderr when no helper exists. + /// + /// Details: + /// - Executes only fake helper scripts inside a temporary directory; no real + /// package operation runs (U7). + fn shell_fallback_helper_preference_and_missing_helper_status() { + let body = aur_install_shell_fallback(&["ripgrep"], &InstallOptions::default()) + .expect("build body"); + let expected_args = ["-S", "--aur", "--needed", "--noconfirm", "--", "ripgrep"]; + + let both = tempfile::tempdir().expect("temporary fixture directory"); + write_fake_helper(both.path(), "paru"); + write_fake_helper(both.path(), "yay"); + let paru_run = run_shell_body(&body, both.path()); + assert!(paru_run.status.success(), "paru branch should succeed"); + assert_eq!(stdout_lines(&paru_run), { + let mut expected = vec!["paru".to_string()]; + expected.extend(expected_args.iter().map(ToString::to_string)); + expected + }); + + let yay_only = tempfile::tempdir().expect("temporary fixture directory"); + write_fake_helper(yay_only.path(), "yay"); + let yay_run = run_shell_body(&body, yay_only.path()); + assert!(yay_run.status.success(), "yay branch should succeed"); + assert_eq!(stdout_lines(&yay_run), { + let mut expected = vec!["yay".to_string()]; + expected.extend(expected_args.iter().map(ToString::to_string)); + expected + }); + + let empty = tempfile::tempdir().expect("temporary fixture directory"); + let missing = run_shell_body(&body, empty.path()); + assert_eq!( + missing.status.code(), + Some(127), + "missing helper must exit non-zero with 127" + ); + assert!( + String::from_utf8_lossy(&missing.stdout).trim().is_empty(), + "missing-helper message must not go to stdout" + ); + assert_eq!( + String::from_utf8_lossy(&missing.stderr).trim(), + NO_AUR_HELPER_MESSAGE + ); + } + + /// What: Write an executable fake AUR helper that records its argv. + /// + /// Inputs: + /// - `dir`: Directory placed on the isolated `PATH`. + /// - `name`: Helper basename (`paru` or `yay`). + /// + /// Output: + /// - Side effect: an executable script printing its own name and arguments. + /// + /// Details: + /// - Keeps helper-preference assertions deterministic without touching the + /// host system. + #[cfg(unix)] + fn write_fake_helper(dir: &std::path::Path, name: &str) { + use std::os::unix::fs::PermissionsExt; + + let path = dir.join(name); + std::fs::write( + &path, + format!("#!/bin/sh\nprintf '%s\\n' \"{name}\" \"$@\"\n"), + ) + .expect("write fake helper"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)) + .expect("mark fake helper executable"); + } + + /// What: Run a shell fallback body with an isolated `PATH`. + /// + /// Inputs: + /// - `body`: Shell snippet produced by a fallback builder. + /// - `path_dir`: Only directory exposed through `PATH`. + /// + /// Output: + /// - Captured `std::process::Output` of `/bin/sh -c `. + /// + /// Details: + /// - `/bin/sh` is invoked by absolute path so the isolated `PATH` cannot + /// affect interpreter lookup. + #[cfg(unix)] + fn run_shell_body(body: &str, path_dir: &std::path::Path) -> std::process::Output { + std::process::Command::new("/bin/sh") + .args(["-c", body]) + .env("PATH", path_dir) + .output() + .expect("shell should run") + } + + /// What: Split captured stdout into trimmed lines. + /// + /// Inputs: + /// - `output`: Process output from a fallback execution. + /// + /// Output: + /// - Owned lines with trailing newline removed. + /// + /// Details: + /// - Used to compare recorded helper argv exactly. + #[cfg(unix)] + fn stdout_lines(output: &std::process::Output) -> Vec { + String::from_utf8_lossy(&output.stdout) + .lines() + .map(ToString::to_string) + .collect() + } + #[test] /// What: Verify update commands for system-wide and helper-driven updates. ///