Validate Safari bundle identifiers by rissrice2105-agent · Pull Request #609 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T18:17:05Z

Fixes #608.

Changes:

validate browser-safari bundleId at runtime before package planning or shipping
normalize the bundleId used for archive paths, converter arguments, logs, and App Store Connect metadata
add regression tests for invalid bundleId in dry-run build and ship flows

Validation:

vitest run packages/targets/browser-safari/src/index.test.ts
tsc -p packages/targets/browser-safari/tsconfig.json --noEmit

greptile-apps · 2026-06-05T18:20:59Z

Greptile Summary

This PR adds runtime validation of Safari extension bundle identifiers via a new requireBundleId helper that trims whitespace and enforces a reverse-DNS regex before any package planning or shipping occurs. It also normalises the validated value across all downstream uses (archive paths, converter arguments, log messages, App Store Connect metadata) and adds two dry-run regression tests.

requireBundleId is called from buildPlan (covering the build path) and directly from ship, so both entry points are consistently guarded before any side effects.
The BUNDLE_ID_PATTERN regex enforces letter-start on every component, which could silently reject valid identifiers whose reverse-DNS origin begins with a digit (e.g. com.3m.MyApp).
config.bundleId.trim() has no null/undefined guard; a missing bundleId would surface a TypeError rather than the descriptive validation message.

Confidence Score: 4/5

Safe to merge; the validation logic is sound and consistently applied across both build and ship paths.

The core change — guarding both entry points with requireBundleId before any side effects — is correct and well-tested. Two small gaps exist: the regex rejects digit-starting reverse-DNS components (e.g. com.3m.App) that Apple's spec would permit, and a missing null guard on .trim() would yield an opaque TypeError instead of the helpful validation message when bundleId is absent at runtime.

packages/targets/browser-safari/src/index.ts — the requireBundleId helper and BUNDLE_ID_PATTERN constant.

Important Files Changed

Filename	Overview
packages/targets/browser-safari/src/index.ts	Adds `requireBundleId` validation (regex + trim) called by both `buildPlan` and `ship`; two minor concerns: no null guard before `.trim()`, and the regex rejects digit-starting reverse-DNS components.
packages/targets/browser-safari/src/index.test.ts	Adds two regression tests covering invalid bundle IDs in dry-run `build` and `ship` paths; temp-dir cleanup is handled correctly in both cases.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called with Config] --> B[requireBundleId]
    B --> C{config.bundleId truthy?}
    C -- No --> D["TypeError: cannot read .trim()"]
    C -- Yes --> E["bundleId = config.bundleId.trim()"]
    E --> F{BUNDLE_ID_PATTERN.test}
    F -- Fail --> G["throw Error: reverse-DNS identifier required"]
    F -- Pass --> H[validated bundleId returned]
    H --> I{build path}
    H --> J{ship path}
    I --> K[buildPlan uses bundleId for archivePath, converter args, plan JSON]
    J --> L{dryRun?}
    L -- Yes --> M[return dry-run id/url]
    L -- No --> N[App Store Connect lookup + xcrun upload]

_{Reviews (1): Last reviewed commit: "Validate Safari bundle identifiers" | Re-trigger Greptile}

greptile-apps · 2026-06-05T18:21:07Z

+const BUNDLE_ID_PATTERN = /^[A-Za-z][A-Za-z0-9-]*(\.[A-Za-z][A-Za-z0-9-]*)+$/;
+
+function requireBundleId(config: Config): string {
+  const bundleId = config.bundleId.trim();


Unchecked .trim() on potentially absent bundleId

config.bundleId.trim() throws a TypeError: Cannot read properties of undefined (reading 'trim') when bundleId is absent or null — even though the TypeScript interface marks it as required, callers using as any or raw JSON configs can omit it at runtime. The resulting error gives no hint about the missing field, unlike the clear reverse-DNS message you get for a malformed value. A simple truthiness guard before trim() would surface a more actionable message in that case.

greptile-apps · 2026-06-05T18:21:08Z

  return value.replace(/[^a-zA-Z0-9._-]+/g, '-').replace(/^-|-$/g, '') || 'safari-extension';
 }

+const BUNDLE_ID_PATTERN = /^[A-Za-z][A-Za-z0-9-]*(\.[A-Za-z][A-Za-z0-9-]*)+$/;


BUNDLE_ID_PATTERN rejects digit-starting DNS components

The regex anchors every component to [A-Za-z] as its first character, so a bundle ID whose reverse-DNS origin starts with a digit — e.g. com.3m.MyApp or com.3shape.Plugin — is rejected even though Apple's bundle ID specification only requires alphanumeric characters, hyphens, and periods. Relaxing the per-component first character to [A-Za-z0-9] better matches the actual spec while still blocking obviously invalid identifiers like bare names without dots or slashes.

Suggested change

const BUNDLE_ID_PATTERN = /^[A-Za-z][A-Za-z0-9-]*(\.[A-Za-z][A-Za-z0-9-]*)+$/;

const BUNDLE_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9-]*(\.[A-Za-z0-9][A-Za-z0-9-]*)+$/;

github-actions · 2026-06-06T07:13:40Z