[rust] Use apple-flat-package crate to extract PKG files

[bonigarcia]

User description

Description

This PR uses an existing crate called apple-flat-package to uncompress PKG artifacts, required to manage Firefox and Edge in macOS.

Motivation and Context

Previously, SM used the shell command pkgutil to this aim. Nevertheless, this extraction is not working properly in GHA (although in local machines seems to continue working).

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

Type

bug_fix, enhancement

---

Description

• Replaced the use of shell commands with the apple_flat_package crate for uncompressing PKG files, enhancing reliability and portability.
• Removed obsolete code and simplified function calls following the new PKG uncompression approach.
• Added apple-flat-package to dependencies for handling PKG file extraction.

---

Changes walkthrough

	Relevant files
Enhancement	files.rs Replace Shell Command with Crate for PKG Uncompression      rust/src/files.rs Replaced shell command-based PKG uncompression with apple_flat_package crate usage. Removed unused major_browser_version parameter from uncompress_pkg function. Added logic to extract PKG files using PkgReader from apple_flat_package. Implemented file extraction and permissions setting for Unix systems. +33/-48  lib.rs Cleanup and Simplification after PKG Uncompression Update rust/src/lib.rs Removed unused constants related to the old PKG uncompression method. Simplified uncompress function calls by removing the major_browser_version parameter. +0/-9
Dependencies	Cargo.toml Add Dependency for PKG File Handling                                          rust/Cargo.toml Added apple-flat-package crate to dependencies. +1/-0

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Description updated to latest commit (683967b)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves integrating a new crate for handling PKG files, replacing shell command logic, and removing obsolete code. The changes are moderate in size and complexity, requiring a reviewer to understand Rust, file handling, and the specific requirements of handling PKG files in macOS.
🧪 Relevant tests	No
🔍 Possible issues	Error Handling: The new uncompress_pkg function could improve its error handling. For example, it assumes the first package in the PKG file is always the correct one to extract without handling cases where this might not be true.
	File Permissions: The code sets file permissions explicitly on Unix systems. This is generally fine, but it should ensure that it doesn't inadvertently set insecure permissions on sensitive files.
	Hardcoded Expectation: The function uncompress_pkg expects at least one package in the PKG file (packages.first().ok_or(anyhow!("Unable to extract PKG"))?;). While this might be a reasonable assumption, it could be more robust against malformed or unexpected PKG files.
🔒 Security concerns	No specific security concerns are introduced in the PR code. However, it's important to ensure that the apple-flat-package crate is securely handling file extraction to avoid vulnerabilities such as path traversal attacks.

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-pro]

PR Code Suggestions

Category	Suggestions
Security	Add path sanitization to prevent directory traversal vulnerabilities. Consider handling the case where entry.name() returns a path that traverses outside the intended directory (e.g., via ../ sequences). This can help prevent potential directory traversal vulnerabilities. You can use canonicalization or path sanitization techniques to ensure that extracted files do not end up outside the target directory. rust/src/files.rs [187-190] let name = entry.name(); +let sanitized_name = sanitize_path(name)?; let mut file = Vec::new(); cpio_reader.read_to_end(&mut file)?; -let target_path_buf = target_path.join(name); +let target_path_buf = target_path.join(sanitized_name);
	Verify the integrity and authenticity of the PKG file before extraction. Consider verifying the integrity and authenticity of the PKG file before extracting it. This can be done by checking digital signatures or checksums. This step is crucial for security, especially if the PKG file comes from an untrusted source. rust/src/files.rs [181] +// Example: Verify the PKG file's signature or checksum before proceeding +verify_pkg_integrity(compressed_file)?; let mut reader = PkgReader::new(File::open(compressed_file)?)?;
Enhancement	Use a library or dedicated function for CPIO extraction. Instead of manually iterating over the cpio_reader and writing files, consider using a more robust library or existing functionality that handles the nuances of CPIO extraction, including directory creation, file metadata, and error handling in a more comprehensive manner. rust/src/files.rs [184-206] if let Some(mut cpio_reader) = package.payload_reader()? { - while let Some(next) = cpio_reader.next() { - let entry = next?; - ... - } + extract_cpio(cpio_reader, target_path)?; }
	Enhance error messages for better debugging and user feedback. To improve error handling, consider providing more context in the error message when unable to extract the PKG. This could include the path of the PKG file being processed. Using anyhow! with a more descriptive error message can aid in debugging and user feedback. rust/src/files.rs [183] -let package = packages.first().ok_or(anyhow!("Unable to extract PKG"))?; +let package = packages.first().ok_or(anyhow!("Unable to extract PKG: {}", compressed_file))?;
Best practice	Add a check for None before unwrapping target_path.parent(). It's a good practice to check if the target_path.parent() is None before unwrapping it to avoid potential panics. Consider handling the case where parent() might return None, which can happen if target_path is a root directory. rust/src/files.rs [194] -fs::create_dir_all(target_path.parent().unwrap())?; +if let Some(parent_path) = target_path.parent() { + fs::create_dir_all(parent_path)?; +} else { + // Handle the case where the target path has no parent (e.g., root directory) +}

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-pro]

CI Failure Feedback

(Checks updated until commit 6e00341)

Action: Rust / Tests (ubuntu) / Tests (ubuntu)

Failed stage: Run Bazel [❌]

Failed test name: //rust:unit

Failure summary: The action failed due to a missing edition attribute in the rust_test rule for the target //rust:unit. This attribute is required for the test to run, as indicated by the error message "Attribute edition is required for @//rust:unit." This issue prevented the analysis of the target //rust:unit, leading to the build being aborted and no test targets being found.

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 963: Package 'php-symfony-debug-bundle' is not installed, so not removed 964: Package 'php-symfony-dependency-injection' is not installed, so not removed 965: Package 'php-symfony-deprecation-contracts' is not installed, so not removed 966: Package 'php-symfony-discord-notifier' is not installed, so not removed 967: Package 'php-symfony-doctrine-bridge' is not installed, so not removed 968: Package 'php-symfony-doctrine-messenger' is not installed, so not removed 969: Package 'php-symfony-dom-crawler' is not installed, so not removed 970: Package 'php-symfony-dotenv' is not installed, so not removed 971: Package 'php-symfony-error-handler' is not installed, so not removed ... 1846: �[32mAnalyzing:�[0m 37 targets (173 packages loaded, 1646 targets configured) 1847: �[32m[1 / 1]�[0m checking cached actions 1848: �[32mAnalyzing:�[0m 37 targets (255 packages loaded, 6644 targets configured) 1849: �[32m[1 / 1]�[0m checking cached actions 1850: �[32mAnalyzing:�[0m 37 targets (372 packages loaded, 10751 targets configured) 1851: �[32m[1 / 1]�[0m checking cached actions 1852: �[32mAnalyzing:�[0m 37 targets (383 packages loaded, 11029 targets configured) 1853: �[32m[1 / 1]�[0m checking cached actions 1854: �[31m�[1mERROR: �[0m/home/runner/work/selenium/selenium/rust/BUILD.bazel:98:10: in rust_test rule //rust:unit: 1855: Traceback (most recent call last): 1856: File "/home/runner/.bazel/external/rules_rust/rust/private/rust.bzl", line 399, column 34, in _rust_test_impl 1857: edition = get_edition(ctx.attr, toolchain, ctx.label), 1858: File "/home/runner/.bazel/external/rules_rust/rust/private/utils.bzl", line 824, column 13, in get_edition 1859: fail("Attribute `edition` is required for {}.".format(label)) 1860: Error in fail: Attribute `edition` is required for @//rust:unit. 1861: �[31m�[1mERROR: �[0m/home/runner/work/selenium/selenium/rust/BUILD.bazel:98:10: Analysis of target '//rust:unit' failed 1862: �[32mAnalyzing:�[0m 37 targets (383 packages loaded, 11037 targets configured) 1863: �[32m[1 / 1]�[0m checking cached actions 1864: Target //rust:enable-rustfmt up-to-date (nothing to build) 1865: �[31m�[1mERROR: �[0mAnalysis of target '//rust:unit' failed; build aborted 1866: �[32mINFO: �[0mElapsed time: 20.689s, Critical Path: 1.50s 1867: �[32mINFO: �[0m1 process: 1 internal. 1868: �[31m�[1mERROR: �[0mBuild did NOT complete successfully 1869: �[31m�[1mFAILED:�[0m 1870: �[31m�[1mERROR: �[0mNo test targets were found, yet testing was requested 1871: �[0m 1872: ##[error]Process completed with exit code 1.

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.

[diemol]

Does the SM binary grow?

[bonigarcia]

This PR fixes the Rust build. In addition to uses the previously described crate to extract PKG files, this PR bumps the version of the rules_rust. We were using a very old version, and that was causing several issues. Now everything seems to work nicely.

Does the SM binary grow?

Not really. You can see the size of the resulting binaries at the bottom of the workflow run:

https://github.com/SeleniumHQ/selenium/actions/runs/8442841426

I don't see differences with respect to the current stable binaries:

https://github.com/SeleniumHQ/selenium/blob/trunk/common/selenium_manager.bzl

[diemol]

Nice, I will merge after CI passes again.

[diemol]

Thank you, @bonigarcia!