fix(ci): eliminate script injection in pr-labeler + add iOS/Android dep patterns (W-08)

[lml2468]

Summary

Security fix (W-08): Eliminates a latent script injection vector in reusable-pr-labeler.yml by replacing direct ${{ inputs.* }} interpolation into JavaScript string literals with the safer env: + process.env.* pattern used by every other reusable workflow in this repo.

Functionality: Extends dependency-change detection to cover iOS (CocoaPods) and Android (Gradle) projects so PRs in octo-ios and octo-android get correctly labeled.

Changes

Security — script injection fix

Before (unsafe — inputs.* interpolated directly into JS):

const owner = '${{ inputs.repo_owner }}';
const repo = '${{ inputs.repo_name }}';
const prNumber = Number('${{ inputs.pr_number }}');

After (safe — passed via env:, read from process.env):

env:
  PR_NUMBER: ${{ inputs.pr_number }}
  REPO_OWNER: ${{ inputs.repo_owner }}
  REPO_NAME: ${{ inputs.repo_name }}

const owner = process.env.REPO_OWNER;
const repo = process.env.REPO_NAME;
const prNumber = parseInt(process.env.PR_NUMBER, 10);

iOS / Android dependency patterns

• LOCKFILES: + Podfile.lock (excluded from size calculation)
• DEP_FILES: + Podfile, Podfile.lock, build.gradle, build.gradle.kts, settings.gradle, settings.gradle.kts

Both changes are backward-compatible; no caller modifications needed.

Test plan

• Trigger the workflow on a PR in octo-ios that modifies Podfile.lock → confirm dependencies-changed label + comment
• Trigger on a PR in octo-android that modifies build.gradle.kts → confirm dependencies-changed label + comment
• Trigger on a Go/JS PR → confirm existing behavior unchanged
• Confirm size labels (size/XS..size/XL) still apply correctly with Podfile.lock excluded from the line count

[lml2468]

Review — PR #54 (.github) @ 1804954

Verdict: APPROVED

Two clean changes in reusable-pr-labeler.yml:

1. 🔴 Script injection fix (security)

Before: '${{ inputs.repo_owner }}' — direct template interpolation inside JS string literals. A crafted repo name or PR number could inject arbitrary JS.
After: inputs passed via env: block → accessed via process.env.* — safe, no injection vector.

This is a textbook fix for the well-known GitHub Actions script injection vulnerability (CWE-94).

2. iOS/Android dependency patterns

• LOCKFILES: added Podfile.lock
• DEP_FILES: added Podfile, Podfile.lock, build.gradle, build.gradle.kts, settings.gradle, settings.gradle.kts

Correct patterns for CocoaPods (iOS) and Gradle (Android). Matches the org's repos (octo-ios, octo-android).

CI all green. No blocking or non-blocking issues.

[Jerry-Xin]

The PR is in scope for Mininglamp-OSS/.github and the workflow change is ready to merge.

💬 Non-blocking

🟡 Warning: .github/workflows/reusable-pr-labeler.yml adds common Gradle build files, but Android projects may also declare dependency versions in gradle/libs.versions.toml. If octo-android uses Gradle version catalogs, changes to that file will not trigger dependencies-changed.

🔵 Suggestion: .github/workflows/reusable-pr-labeler.yml still excludes package-lock.json from size calculation but does not include it in DEP_FILES. This appears pre-existing, but it is worth aligning while this dependency detection list is being updated.

✅ Highlights

🟢 The direct ${{ inputs.* }} interpolation into JavaScript has been removed and replaced with env plus process.env, matching safer patterns used elsewhere in the repository.

🟢 Podfile.lock is now excluded from PR size calculation while still triggering dependency-change labeling, which is the right behavior for lockfile-heavy iOS PRs.

[yujiawei]

Code Review — PR #54 (.github)

Summary

This PR makes two backward-compatible changes to the reusable PR-labeler workflow:

1. Security: removes a script-injection vector by passing inputs.* through env: and reading them with process.env.* inside actions/github-script, instead of interpolating ${{ inputs.* }} directly into the JS source.
2. Functionality: extends dependency-change detection to iOS (CocoaPods) and Android (Gradle) files.

Verdict: Approve. The security fix is correct and complete for the affected script block, and the functional additions are sound. No blocking issues; a few non-blocking observations below.

1. Verification

Security — script injection fix ✅

.github/workflows/reusable-pr-labeler.yml:32-46

Before, the values were interpolated into JS string literals at template-expansion time, before the script ran:

const owner = '${{ inputs.repo_owner }}';
const prNumber = Number('${{ inputs.pr_number }}');

A value containing a single quote (e.g. foo'); <code>; //) would terminate the string literal and execute arbitrary code on the runner. This is the canonical GitHub Actions script-injection class.

After, the values are supplied as env: entries and read at runtime via process.env.*. Environment-variable values are never parsed as source, so the injection vector is eliminated. This is exactly the GitHub-recommended mitigation and matches the env: + process.env pattern used elsewhere in this repo. ✅

I also confirmed there is no residual interpolation left in the script: block — owner, repo, and prNumber are the only inputs and all three now come from process.env. The env: assignments (PR_NUMBER: ${{ inputs.pr_number }} etc.) are themselves the safe form: interpolation into a YAML env value is not code-injectable.

Supply-chain / least-privilege (unchanged, noted for the security-sensitive classification) ✅

• actions/github-script is pinned to a full commit SHA (3a2844b… / v9.0.0). ✅
• Top-level permissions: {} with job-scoped issues: write + pull-requests: write only. Least privilege is preserved; this PR does not widen permissions. ✅

iOS / Android dependency patterns ✅

.github/workflows/reusable-pr-labeler.yml:38-46

• LOCKFILES: + Podfile.lock — correctly excluded from the size line count.
• DEP_FILES: + Podfile, Podfile.lock, build.gradle, build.gradle.kts, settings.gradle, settings.gradle.kts.

Detection matches on f.filename.split('/').pop() (basename), so nested module files like app/build.gradle.kts are matched correctly in monorepo/multi-module layouts. ✅

Behavior change: Number(...) → parseInt(..., 10) ✅

For the number-typed, required pr_number input the result is identical. parseInt is slightly more lenient on trailing garbage, but that input is workflow-controlled and well-formed, so this is a non-issue.

2. Issues

None at P0/P1. No correctness, security, data-loss, or build-break concerns.

3. Non-blocking observations (nits / future cleanup)

• package-lock.json asymmetry (pre-existing, not introduced here): package-lock.json is in LOCKFILES (excluded from size) but absent from DEP_FILES, so an npm lockfile-only change won't trigger the dependencies-changed label, unlike pnpm-lock.yaml/yarn.lock. Worth aligning in a follow-up, but out of scope for this PR.
• Gradle has no lockfile entry: build.gradle* was added to DEP_FILES but not LOCKFILES; that's reasonable since Gradle lockfiles (gradle.lockfile) are uncommon. Just flagging the intent.

4. Manual-verification note for a human (security-sensitive PR)

The fix is sound at the code level. The one thing worth a human glance is the call sites: confirm that callers of this reusable workflow pass trusted/org-controlled values for repo_owner/repo_name/pr_number (they normally are). The env: change makes injection a non-issue regardless, so this is confirmation-only, not a blocker.