starred/App-Store-Connect-CLI

Fork 0

mirror of https://github.com/rudrankriyam/App-Store-Connect-CLI.git synced 2026-04-25 07:35:48 +03:00

[GH-ISSUE #523] Publish hardening: reject symlink IPA paths to prevent unintended uploads #147

New issue

Closed

opened 2026-02-26 21:33:46 +03:00 by kerem · 3 comments

kerem commented

2026-02-26 21:33:46 +03:00

Owner

Originally created by @rudrankriyam on GitHub (Feb 14, 2026).
Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/523

Summary

publish IPA validation follows symlinks (os.Stat + os.Open), allowing an unintended target file to be uploaded via symlink path.

Why this matters

Upload commands should be explicit and safe. Following symlinks can lead to accidental upload of the wrong artifact.

Current behavior

internal/cli/publish/publish.go uses os.Stat in validateIPAPath.
internal/asc/upload.go uses os.Open in ExecuteUploadOperations.

Expected behavior

Either reject symlink IPA paths, or safely open with no-follow semantics where supported.
Return clear error if input path is a symlink.

Reproduction

Create symlink: ln -s /path/to/other.ipa app.ipa

Run publish:

asc publish testflight --app "APP_ID" --ipa app.ipa --group "GROUP_ID"

Upload uses symlink target unexpectedly.

Detailed implementation plan

Update IPA path validation to use os.Lstat and reject symlink mode.
Reuse secure file-open helper where possible for upload source reads.
Ensure error message clearly explains symlink rejection.
Add tests for symlink input on supported platforms.

Acceptance criteria

Symlink IPA paths are rejected before upload operations begin.
Regular IPA files continue to work unchanged.
Error messaging is actionable and deterministic.

cc @cursor please implement this end-to-end.

Originally created by @rudrankriyam on GitHub (Feb 14, 2026). Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/523 ## Summary `publish` IPA validation follows symlinks (`os.Stat` + `os.Open`), allowing an unintended target file to be uploaded via symlink path. ## Why this matters Upload commands should be explicit and safe. Following symlinks can lead to accidental upload of the wrong artifact. ## Current behavior - `internal/cli/publish/publish.go` uses `os.Stat` in `validateIPAPath`. - `internal/asc/upload.go` uses `os.Open` in `ExecuteUploadOperations`. ## Expected behavior - Either reject symlink IPA paths, or safely open with no-follow semantics where supported. - Return clear error if input path is a symlink. ## Reproduction 1. Create symlink: `ln -s /path/to/other.ipa app.ipa` 2. Run publish: ```bash asc publish testflight --app "APP_ID" --ipa app.ipa --group "GROUP_ID" ``` 3. Upload uses symlink target unexpectedly. ## Detailed implementation plan - [ ] Update IPA path validation to use `os.Lstat` and reject symlink mode. - [ ] Reuse secure file-open helper where possible for upload source reads. - [ ] Ensure error message clearly explains symlink rejection. - [ ] Add tests for symlink input on supported platforms. ## Acceptance criteria - [ ] Symlink IPA paths are rejected before upload operations begin. - [ ] Regular IPA files continue to work unchanged. - [ ] Error messaging is actionable and deterministic. cc @cursor please implement this end-to-end.

kerem

2026-02-26 21:33:46 +03:00

closed this issue
added the
bug
label

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

@cursor please implement this issue end-to-end using the checklist and acceptance criteria.

@rudrankriyam commented on GitHub (Feb 14, 2026): @cursor please implement this issue end-to-end using the checklist and acceptance criteria.

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@cursor[bot] commented on GitHub (Feb 14, 2026):

Implemented end-to-end on cursor/ipa-symlink-rejection-a2e3 and pushed commit 8e7819b (Harden publish IPA handling against symlink paths).

Symlink rejection in publish validation: internal/cli/publish/publish.go now uses os.Lstat in validateIPAPath and explicitly errors on symlinks (refusing to read symlink "...") before any upload work starts.
Hardened upload source open path: internal/asc/upload.go now opens source files through a new openUploadSourceFile flow that checks symlinks and uses a no-follow open helper.
No-follow helper added in asc package:
- internal/asc/secure_open_unix.go uses O_NOFOLLOW on Unix platforms.
- internal/asc/secure_open_other.go keeps platform-compatible fallback behavior.
Tests added/updated:
- internal/cli/publish/publish_test.go (reject symlink, allow regular file)
- internal/asc/upload_test.go (reject symlink upload source)
- internal/cli/cmdtest/commands_test.go (CLI-level regression for both publish testflight and publish appstore symlink IPA paths)

Validation I ran (with runtime evidence):

TDD red → green: new symlink tests failed before fix, then passed after fix.
Repo gates: make format, make lint, make test all pass.
Black-box built binary exit behavior (as requested by repo checklist):
- /tmp/asc publish testflight ... --ipa <symlink> ... → Error: publish testflight: refusing to read symlink "...", exit_code=1
- /tmp/asc publish appstore ... --ipa <symlink> ... → Error: publish appstore: refusing to read symlink "...", exit_code=1

@cursor[bot] commented on GitHub (Feb 14, 2026): Implemented end-to-end on `cursor/ipa-symlink-rejection-a2e3` and pushed commit `8e7819b` (`Harden publish IPA handling against symlink paths`). - **Symlink rejection in publish validation:** `internal/cli/publish/publish.go` now uses `os.Lstat` in `validateIPAPath` and explicitly errors on symlinks (`refusing to read symlink "..."`) before any upload work starts. - **Hardened upload source open path:** `internal/asc/upload.go` now opens source files through a new `openUploadSourceFile` flow that checks symlinks and uses a no-follow open helper. - **No-follow helper added in asc package:** - `internal/asc/secure_open_unix.go` uses `O_NOFOLLOW` on Unix platforms. - `internal/asc/secure_open_other.go` keeps platform-compatible fallback behavior. - **Tests added/updated:** - `internal/cli/publish/publish_test.go` (reject symlink, allow regular file) - `internal/asc/upload_test.go` (reject symlink upload source) - `internal/cli/cmdtest/commands_test.go` (CLI-level regression for both `publish testflight` and `publish appstore` symlink IPA paths) Validation I ran (with runtime evidence): - **TDD red → green:** new symlink tests failed before fix, then passed after fix. - **Repo gates:** `make format`, `make lint`, `make test` all pass. - **Black-box built binary exit behavior (as requested by repo checklist):** - `/tmp/asc publish testflight ... --ipa <symlink> ...` → `Error: publish testflight: refusing to read symlink "..."`, `exit_code=1` - `/tmp/asc publish appstore ... --ipa <symlink> ...` → `Error: publish appstore: refusing to read symlink "..."`, `exit_code=1` <p><a href="https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/531"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/view-pr-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/view-pr-light.png"><img alt="View PR" width="86" height="28" src="https://cursor.com/assets/images/view-pr-light.png"></picture></a></p> <p><a href="https://cursor.com/background-agent?bcId=bc-bc9ee075-7cfd-4b93-824b-cbd05febcabb"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> <a href="https://cursor.com/agents?id=bc-bc9ee075-7cfd-4b93-824b-cbd05febcabb"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a></p>

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

Fixed in #531 (merged to main), with non-Unix follow-up in #536.

@rudrankriyam commented on GitHub (Feb 14, 2026): Fixed in #531 (merged to main), with non-Unix follow-up in #536.

kerem referenced this issue

2026-02-26 21:34:35 +03:00

[PR #148] [MERGED] Add TestFlight 'What to Test' notes (betaBuildLocalizations) support #328