starred/App-Store-Connect-CLI

Fork 0

mirror of https://github.com/rudrankriyam/App-Store-Connect-CLI.git synced 2026-04-25 23:55:51 +03:00

[GH-ISSUE #524] Cross-platform hardening: secure_open_other should avoid symlink following where possible #150

New issue

Closed

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

kerem commented

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

Owner

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

Summary

internal/cli/shared/secure_open_other.go falls back to plain os.Open/os.OpenFile on non-Unix builds, which can follow symlinks and weakens the no-follow guarantees present on Unix builds.

Why this matters

Security posture is currently platform-dependent. For report/artifact file paths, symlink behavior should be explicitly hardened or documented consistently.

Current behavior

Unix builds: O_NOFOLLOW is used.
Other builds: no equivalent no-follow check is applied.

Expected behavior

Implement best-effort symlink protection on non-Unix platforms, or
perform explicit Lstat checks + controlled open sequence, and
clearly document any unavoidable limitations.

Detailed implementation plan

Evaluate platform-specific no-follow capabilities.
Add explicit symlink checks before open/create in secure_open_other.go.
Ensure TOCTOU risk is minimized and documented.
Add tests covering symlink path handling on non-Unix-compatible test strategy.
Update security notes/docs for platform differences.

Acceptance criteria

Non-Unix fallback has explicit, tested symlink-handling behavior.
Behavior is documented and consistent with security expectations.
Existing file write/read helpers continue to pass existing tests.

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/524 ## Summary `internal/cli/shared/secure_open_other.go` falls back to plain `os.Open`/`os.OpenFile` on non-Unix builds, which can follow symlinks and weakens the no-follow guarantees present on Unix builds. ## Why this matters Security posture is currently platform-dependent. For report/artifact file paths, symlink behavior should be explicitly hardened or documented consistently. ## Current behavior - Unix builds: `O_NOFOLLOW` is used. - Other builds: no equivalent no-follow check is applied. ## Expected behavior - Implement best-effort symlink protection on non-Unix platforms, or - perform explicit `Lstat` checks + controlled open sequence, and - clearly document any unavoidable limitations. ## Detailed implementation plan - [ ] Evaluate platform-specific no-follow capabilities. - [ ] Add explicit symlink checks before open/create in `secure_open_other.go`. - [ ] Ensure TOCTOU risk is minimized and documented. - [ ] Add tests covering symlink path handling on non-Unix-compatible test strategy. - [ ] Update security notes/docs for platform differences. ## Acceptance criteria - [ ] Non-Unix fallback has explicit, tested symlink-handling behavior. - [ ] Behavior is documented and consistent with security expectations. - [ ] Existing file write/read helpers continue to pass existing tests. cc @cursor please implement this end-to-end.

kerem

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

closed this issue
added the
enhancement
label

kerem commented

2026-02-26 21:33:48 +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:48 +03:00

Author

Owner

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

Implemented end-to-end on cursor/cross-platform-symlink-security-cd97 and pushed the changes. I replaced the non-Unix fallback’s plain open calls with a best-effort hardened flow: pre-open Lstat symlink rejection + post-open verification that the opened fd still matches the path (to detect path swaps/races), with explicit TOCTOU caveats in code comments. I wired this through internal/cli/shared/secure_open_other.go and added shared helper logic in internal/cli/shared/secure_open_best_effort.go.

I added/updated tests to satisfy the checklist:

New coverage in internal/cli/shared/secure_open_best_effort_test.go for:
- symlink rejection on open-existing and open-new paths
- detection of path/fd mismatch (race/path-swap guard)
- regular-file success paths
Updated internal/cli/shared/junit_report_test.go symlink test to use capability-based skip (symlink not supported) instead of unconditional Windows skip, so behavior is checked consistently where symlinks are available.
Updated docs in README.md Security section to document Unix (O_NOFOLLOW) vs non-Unix best-effort behavior and residual TOCTOU limitation.

Validation evidence (all run successfully):

go test ./internal/cli/shared -run 'TestOpen(Existing|New)NoFollowBestEffort|TestJUnitReport_WriteRefusesSymlink|TestReadLocalizationStrings_RejectsSymlink'
make format
make lint
make test
GOOS=windows GOARCH=amd64 go test ./internal/cli/shared -c -o /tmp/shared_windows.test.exe (cross-platform build sanity for non-Unix path)
Commits pushed:
- aad5d95 Harden non-unix secure open with best-effort nofollow checks
- 37d7b1a Format secure open best-effort helper

@cursor[bot] commented on GitHub (Feb 14, 2026): Implemented end-to-end on `cursor/cross-platform-symlink-security-cd97` and pushed the changes. I replaced the non-Unix fallback’s plain open calls with a best-effort hardened flow: pre-open `Lstat` symlink rejection + post-open verification that the opened fd still matches the path (to detect path swaps/races), with explicit TOCTOU caveats in code comments. I wired this through `internal/cli/shared/secure_open_other.go` and added shared helper logic in `internal/cli/shared/secure_open_best_effort.go`. I added/updated tests to satisfy the checklist: - New coverage in `internal/cli/shared/secure_open_best_effort_test.go` for: - symlink rejection on open-existing and open-new paths - detection of path/fd mismatch (race/path-swap guard) - regular-file success paths - Updated `internal/cli/shared/junit_report_test.go` symlink test to use capability-based skip (`symlink not supported`) instead of unconditional Windows skip, so behavior is checked consistently where symlinks are available. - Updated docs in `README.md` Security section to document Unix (`O_NOFOLLOW`) vs non-Unix best-effort behavior and residual TOCTOU limitation. Validation evidence (all run successfully): - `go test ./internal/cli/shared -run 'TestOpen(Existing|New)NoFollowBestEffort|TestJUnitReport_WriteRefusesSymlink|TestReadLocalizationStrings_RejectsSymlink'` - `make format` - `make lint` - `make test` - `GOOS=windows GOARCH=amd64 go test ./internal/cli/shared -c -o /tmp/shared_windows.test.exe` (cross-platform build sanity for non-Unix path) - Commits pushed: - `aad5d95` Harden non-unix secure open with best-effort nofollow checks - `37d7b1a` Format secure open best-effort helper <p><a href="https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/532"><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-856d6e41-eefa-4d77-b57c-0afd9af403c6"><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-856d6e41-eefa-4d77-b57c-0afd9af403c6"><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:48 +03:00

Author

Owner

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

Fixed in #532 (merged to main).

@rudrankriyam commented on GitHub (Feb 14, 2026): Fixed in #532 (merged to main).

kerem referenced this issue

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

[PR #150] [MERGED] Builds upload wait timeout #329