Skip to content

fix fdroid package name validation#559

Open
hhyunbreh wants to merge 2 commits into
profullstack:masterfrom
hhyunbreh:codex/fix-from-forge-subpaths
Open

fix fdroid package name validation#559
hhyunbreh wants to merge 2 commits into
profullstack:masterfrom
hhyunbreh:codex/fix-from-forge-subpaths

Conversation

@hhyunbreh
Copy link
Copy Markdown

Summary

  • validate pkg-fdroid Android package names before build and ship work
  • reject malformed names before they can be used in metadata artifact filenames
  • add regression coverage for path separators and malformed package segments

Why

pkg-fdroid used config.packageName directly in the generated metadata filename. Invalid values such as ../escape or com.acme/app should be rejected instead of influencing the artifact path.

Tests

  • vitest run packages/targets/pkg-fdroid/src/index.test.ts passed
  • tsc -p packages/targets/pkg-fdroid/tsconfig.json --noEmit passed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 3, 2026

Greptile Summary

This PR adds an assertPackageName validator to the F-Droid target that guards both build and ship entry points, preventing malformed Android package names from reaching file-system operations or metadata paths.

  • Introduces a combined regex + JAVA_RESERVED_WORDS check that rejects path-traversal sequences (../escape), path-separator characters (com.acme/app), empty segments (com..acme), digit-leading names (1com.acme.app), and Java-reserved-keyword segments (com.int.app).
  • Adds two regression tests (one for build, one for ship) that iterate over a shared invalidPackageNames fixture and assert that each throws before any file I/O occurs.

Confidence Score: 5/5

Safe to merge — the validation is correctly placed before all file I/O and covers the full set of malformed inputs described in the PR.

The assertPackageName guard fires at the very top of both build and ship, before any path construction or disk write. The regex correctly rejects path traversal, embedded slashes, empty segments, digit-leading names, and single-segment names. The JAVA_RESERVED_WORDS set is complete for Java 8 (including literals true, false, null that aapt also rejects). All five invalid fixtures in the test array are correctly rejected by the implementation, and com.int.app confirms the keyword-segment fix raised in earlier review is now addressed.

No files require special attention.

Important Files Changed

Filename Overview
packages/targets/pkg-fdroid/src/index.ts Adds assertPackageName guard at the top of both build and ship, backed by a regex + a complete JAVA_RESERVED_WORDS set; called before any file I/O or side-effecting operations.
packages/targets/pkg-fdroid/src/index.test.ts Adds two regression tests (build + ship) covering path-traversal, path-separators, double-dots, digit-leading, and Java-reserved-keyword segments; uses a shared invalidPackageNames fixture array.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["build / ship called"] --> B["assertPackageName(packageName)"]
    B --> C{"regex matches?"}
    C -- No --> E["throw Error: invalid packageName"]
    C -- Yes --> D{"segment in JAVA_RESERVED_WORDS?"}
    D -- Yes --> E
    D -- No --> F["continue build/ship logic"]
    F --> G{"mode?"}
    G -- "main-repo" --> H["write metadata YAML or open fdroiddata PR"]
    G -- "self-hosted" --> I["fdroid update or publish repo"]
Loading

Reviews (2): Last reviewed commit: "address fdroid package name review" | Re-trigger Greptile

Comment thread packages/targets/pkg-fdroid/src/index.ts
Comment thread packages/targets/pkg-fdroid/src/index.ts
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.

1 participant