Skip to content

fix(link): validate pkg_config names before invoking pkg-config#5073

Merged
proggeramlug merged 2 commits into
mainfrom
fix/pkg-config-name-validation
Jun 13, 2026
Merged

fix(link): validate pkg_config names before invoking pkg-config#5073
proggeramlug merged 2 commits into
mainfrom
fix/pkg-config-name-validation

Conversation

@TheHypnoo

@TheHypnoo TheHypnoo commented Jun 13, 2026

Copy link
Copy Markdown
Member

What

Adds crates/perry/src/commands/compile/link/pkg_config.rs with a conservative allowlist validator, and calls it before both pkg-config invocation sites in link/mod.rs.

Why

pkg_config entries from the user's package.json (perry.nativeLibrary…) were passed straight to Command::new("pkg-config").args(["--libs", pkg]) with no validation. No shell is spawned, so this is argument-confusion (e.g. a leading - parsed as a pkg-config flag) rather than shell injection, and the input is the user's own config — so severity is low. Validating against ^[A-Za-z0-9._/@+-]+$ (rejecting empty / leading -) removes the argument-injection surface and turns a confusing link failure into an actionable error naming the offending entry. All legitimate package names that work today keep working.

Test

4 new unit tests (accepts real names like openssl / gtk+-3.0; rejects empty, leading-dash flags, whitespace/metacharacters). cargo test -p perry pkg_config → 4 passed.

Closes #5068

Summary by CodeRabbit

  • Bug Fixes
    • Added validation for pkg-config package names during compilation, now catching invalid inputs (empty strings, flag-like patterns, unsupported characters) early with clear error messages instead of silently proceeding.

@TheHypnoo TheHypnoo self-assigned this Jun 13, 2026
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ccb11499-d4f0-4e6e-87e8-a0b193ebd94f

📥 Commits

Reviewing files that changed from the base of the PR and between ab5b7e8 and 5c50c55.

📒 Files selected for processing (2)
  • crates/perry/src/commands/compile/link/mod.rs
  • crates/perry/src/commands/compile/link/pkg_config.rs

📝 Walkthrough

Walkthrough

This pull request adds input validation for pkg-config package names sourced from user configuration before passing them to the pkg-config command. A new validator function rejects empty strings, leading-dash prefixes, and characters outside a conservative allowlist, and validation is integrated at two points in the linking flow.

Changes

pkg-config package name validation

Layer / File(s) Summary
Validator function and tests
crates/perry/src/commands/compile/link/pkg_config.rs
validate_pkg_config_name function rejects empty input, leading - characters, and any non-allowlisted characters (alphanumerics, ., _, /, @, +, -), returning descriptive anyhow errors. Comprehensive unit tests assert acceptance of real-world package names and rejection of empty strings, flag-like inputs, and whitespace/metacharacter cases.
Integration in link module
crates/perry/src/commands/compile/link/mod.rs
New pkg_config module is declared. Validation is inserted before pkg-config invocation for target_config.pkg_config entries and for each backend's pkg_config entries, propagating errors on invalid names.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through the pkg-config glen,
Checking each name before the invocation pen.
No dashes, no spaces—just alphanumeric cheer,
Validation's a friend, keeping builds crystal-clear!
Early errors now bloom, like carrots so bright. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding validation for pkg_config names before invoking pkg-config.
Description check ✅ Passed The description adequately covers what changed, why it matters, and includes test evidence, though it does not explicitly follow all template sections.
Linked Issues check ✅ Passed The PR fully implements the requirements from issue #5068: validates pkg_config names against conservative allowlist, rejects leading dashes and metacharacters, and includes unit tests.
Out of Scope Changes check ✅ Passed All changes are in scope: adds validation module and integrates it at both pkg-config invocation sites as required by #5068.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pkg-config-name-validation

Comment @coderabbitai help to get the list of available commands and usage tips.

@proggeramlug proggeramlug merged commit e691a8a into main Jun 13, 2026
14 checks passed
@proggeramlug proggeramlug deleted the fix/pkg-config-name-validation branch June 13, 2026 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate pkg_config package names from package.json before passing to pkg-config

2 participants