Serialize xcodebuild and fetch prebuilt GhosttyKit to unblock Xcode 26 builds #2981
+206
−5
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| from pathlib import PurePosixPath | ||
| import sys | ||
| import tarfile | ||
|
|
||
|
|
||
| ROOT = "GhosttyKit.xcframework" | ||
|
|
||
|
|
||
| def normalize(name: str) -> str: | ||
| while name.startswith("./"): | ||
| name = name[2:] | ||
| return name | ||
|
|
||
|
|
||
| def is_safe_member(name: str) -> bool: | ||
| path = PurePosixPath(name) | ||
| return not path.is_absolute() and ".." not in path.parts | ||
|
|
||
|
|
||
| def main() -> None: | ||
| if len(sys.argv) != 2: | ||
| raise SystemExit("usage: validate-xcframework-archive.py <archive>") | ||
|
|
||
| archive = sys.argv[1] | ||
| with tarfile.open(archive, "r:gz") as tar: | ||
| saw_root = False | ||
| for member in tar.getmembers(): | ||
| name = normalize(member.name) | ||
| if not is_safe_member(name): | ||
| raise SystemExit(f"unsafe archive entry: {member.name}") | ||
| if name != ROOT and not name.startswith(ROOT + "/"): | ||
| raise SystemExit(f"unexpected archive entry: {member.name}") | ||
| if name == ROOT or name == ROOT + "/": | ||
|
There was a problem hiding this comment. P2: The validator incorrectly requires an explicit root directory entry, so valid archives with only Prompt for AI agents
|
||
| saw_root = True | ||
| if member.islnk() or member.issym(): | ||
| target = normalize(member.linkname) | ||
| if not target or not is_safe_member(target): | ||
| raise SystemExit(f"unsafe archive link target: {member.linkname}") | ||
|
Comment on lines
+35
to
+40
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: cat scripts/validate-xcframework-archive.pyRepository: manaflow-ai/cmux Length of output: 1572 🏁 Script executed: #!/bin/bash
# Check if there are any test files that show expected behavior
find . -type f -name "*test*validate*" -o -name "*tar*test*" 2>/dev/null | head -20
# Also check git history for context on this file
git log --oneline scripts/validate-xcframework-archive.py 2>/dev/null | head -5
# Search for how this archive is created
git grep -l "xcframework.tar.gz\|validate-xcframework" 2>/dev/nullRepository: manaflow-ai/cmux Length of output: 341 🏁 Script executed: # Check the download and ensure scripts to understand archive usage
cat scripts/download-prebuilt-ghosttykit.sh
echo "---"
cat scripts/ensure-ghosttykit.shRepository: manaflow-ai/cmux Length of output: 11270 🏁 Script executed: #!/bin/bash
# Verify the key security claims about the link validation logic
# Create a test script to check path safety logic
python3 << 'VERIFY'
from pathlib import PurePosixPath
# Current validation function
def is_safe_member(name: str) -> bool:
path = PurePosixPath(name)
return not path.is_absolute() and ".." not in path.parts
# Test cases for the review's concerns
print("=== Testing current is_safe_member() validation ===\n")
# Test 1: Hardlink target outside ROOT with no ".." or absolute path
print("Test 1: Hardlink to 'some-other-path'")
target = "some-other-path"
safe = is_safe_member(target)
print(f" is_safe_member('{target}'): {safe}")
print(f" Issue: This path doesn't start with ROOT but is_safe_member returns {safe}\n")
# Test 2: Relative path with ".."
print("Test 2: Path with '..'")
target = "../escape"
safe = is_safe_member(target)
print(f" is_safe_member('{target}'): {safe}\n")
# Test 3: Absolute path
print("Test 3: Absolute path")
target = "/etc/passwd"
safe = is_safe_member(target)
print(f" is_safe_member('{target}'): {safe}\n")
# Test 4: Relative path that looks safe but could escape via symlink resolution
print("Test 4: Relative symlink in subdirectory")
symlink_name = "GhosttyKit.xcframework/subdir/link"
symlink_target = "foo"
safe = is_safe_member(symlink_target)
print(f" Member path: {symlink_name}")
print(f" Link target: {symlink_target}")
print(f" is_safe_member('{symlink_target}'): {safe}")
symlink_parent = PurePosixPath(symlink_name).parent
resolved = symlink_parent / symlink_target
print(f" Resolved target when extracted: {resolved}")
print(f" Stays under ROOT? {str(resolved).startswith('GhosttyKit.xcframework')}\n")
# Test 5: ROOT as a symlink
print("Test 5: ROOT entry as symlink")
root_name = "GhosttyKit.xcframework"
root_target = "../real"
safe = is_safe_member(root_target)
print(f" Member path: {root_name}")
print(f" Link target: {root_target}")
print(f" Current code rejects with '..'? {'..' in PurePosixPath(root_target).parts}")
print(f" But if target was 'real'? {is_safe_member('real')}")
print(f" Issue: ROOT as symlink could redirect framework access\n")
print("=== Summary of issues ===")
print("1. Hardlinks can target paths outside ROOT (e.g., 'some-other-path')")
print("2. ROOT itself could be a symlink to a relative target without '..'")
print("3. Symlinks in subdirs could resolve outside ROOT if we only validate target safety, not resolution")
VERIFYRepository: manaflow-ai/cmux Length of output: 1109 🏁 Script executed: #!/bin/bash
# Verify the proposed fix logic
python3 << 'VERIFY'
from pathlib import PurePosixPath
ROOT = "GhosttyKit.xcframework"
def normalize(name: str) -> str:
while name.startswith("./"):
name = name[2:]
return name
def is_safe_member(name: str) -> bool:
path = PurePosixPath(name)
return not path.is_absolute() and ".." not in path.parts
# Proposed fix helper
def is_under_root(path: PurePosixPath) -> bool:
value = str(path)
return value == ROOT or value.startswith(ROOT + "/")
print("=== Testing proposed fix logic ===\n")
# Test case 1: ROOT as hardlink to relative target
print("Test 1: ROOT as hardlink")
name = "GhosttyKit.xcframework"
member_linkname = "something"
target = normalize(member_linkname)
if name.rstrip("/") == ROOT:
print(f" Member: {name}")
print(f" Target: {target}")
print(f" Proposed fix REJECTS: 'archive root must be a directory'\n")
# Test case 2: Symlink in subdirectory with absolute resolution
print("Test 2: Symlink in subdir - absolute resolution")
name = "GhosttyKit.xcframework/lib"
member_linkname = "external"
target = normalize(member_linkname)
target_path = PurePosixPath(target)
name_path = PurePosixPath(name)
resolved_target = name_path.parent / target_path
print(f" Member: {name}")
print(f" Target: {target}")
print(f" Name parent: {name_path.parent}")
print(f" Resolved: {resolved_target}")
print(f" is_under_root({resolved_target}): {is_under_root(resolved_target)}")
if not is_under_root(resolved_target):
print(f" Proposed fix REJECTS: 'escapes ROOT'\n")
# Test case 3: Symlink with relative path back out
print("Test 3: Symlink pointing outside")
name = "GhosttyKit.xcframework/subdir/lib"
member_linkname = "../../external"
target = normalize(member_linkname)
if ".." in PurePosixPath(target).parts:
print(f" Member: {name}")
print(f" Target: {target}")
print(f" Contains '..': Yes")
print(f" Already caught by is_safe_member check\n")
# Test case 4: Hardlink to outside member
print("Test 4: Hardlink validation")
name = "GhosttyKit.xcframework/libghost.a"
member_linkname = "other-lib"
target = normalize(member_linkname)
target_path = PurePosixPath(target)
resolved_target = target_path # For hardlinks, no relative resolution
print(f" Member: {name}")
print(f" Hardlink target: {target}")
print(f" Resolved (hardlink, no parent): {resolved_target}")
print(f" is_under_root({resolved_target}): {is_under_root(resolved_target)}")
if not is_under_root(resolved_target):
print(f" Proposed fix REJECTS: 'escapes ROOT'\n")
# Test case 5: Valid symlink within framework
print("Test 5: Valid symlink within framework")
name = "GhosttyKit.xcframework/lib/link"
member_linkname = "libghostty.a"
target = normalize(member_linkname)
target_path = PurePosixPath(target)
name_path = PurePosixPath(name)
resolved_target = name_path.parent / target_path
print(f" Member: {name}")
print(f" Target: {target}")
print(f" Resolved: {resolved_target}")
print(f" is_under_root({resolved_target}): {is_under_root(resolved_target)}")
print(f" Result: ALLOWED\n")
print("=== Fix correctness assessment ===")
print("✓ Prevents ROOT from being a symlink/hardlink")
print("✓ Ensures symlink targets resolve within ROOT (relative resolution)")
print("✓ Ensures hardlink targets are within ROOT")
VERIFYRepository: manaflow-ai/cmux Length of output: 1288 🏁 Script executed: #!/bin/bash
# Verify edge cases and implementation correctness of the proposed fix
python3 << 'VERIFY'
from pathlib import PurePosixPath
ROOT = "GhosttyKit.xcframework"
def is_under_root(path: PurePosixPath) -> bool:
value = str(path)
return value == ROOT or value.startswith(ROOT + "/")
print("=== Testing edge cases in proposed fix ===\n")
# Edge case 1: name.rstrip("/") logic
print("Edge case 1: ROOT with trailing slash")
names = ["GhosttyKit.xcframework", "GhosttyKit.xcframework/"]
for name in names:
result = name.rstrip("/") == ROOT
print(f" name.rstrip('/') == ROOT for '{name}': {result}")
print()
# Edge case 2: Path with PurePosixPath normalization
print("Edge case 2: PurePosixPath path normalization")
test_paths = [
("GhosttyKit.xcframework", True),
("GhosttyKit.xcframework/", True),
("GhosttyKit.xcframework/subdir", True),
("GhosttyKit.xcframework/subdir/file", True),
("other-dir", False),
("other-dir/GhosttyKit.xcframework", False),
]
for path_str, expected in test_paths:
result = is_under_root(PurePosixPath(path_str))
status = "✓" if result == expected else "✗"
print(f" {status} is_under_root('{path_str}'): {result} (expected {expected})")
print()
# Edge case 3: Symlink resolution when name ends with /
print("Edge case 3: Symlink resolution with trailing slash in name")
name = "GhosttyKit.xcframework/lib/"
target = "external"
target_path = PurePosixPath(target)
name_path = PurePosixPath(name)
resolved = name_path.parent / target_path
print(f" name: '{name}'")
print(f" name_path.parent: {name_path.parent}")
print(f" resolved: {resolved}")
print(f" Correct? {str(resolved) == 'GhosttyKit.xcframework/external'}\n")
# Edge case 4: Hardlink with relative target
print("Edge case 4: Hardlink to relative path")
target = "lib/libghostty.a"
target_path = PurePosixPath(target)
under = is_under_root(target_path)
print(f" Hardlink target: '{target}'")
print(f" is_under_root: {under}")
print(f" Correctly REJECTED: {not under}\n")
# Edge case 5: Check if all member names are already validated to start with ROOT
print("Edge case 5: Pre-validation of member names")
print(" From code: every member name is checked with:")
print(" if name != ROOT and not name.startswith(ROOT + '/'):")
print(" raise SystemExit(...)")
print(" So at link validation, name ALWAYS satisfies is_under_root(name)")
print(" Therefore, for symlinks, resolved_target will too (if it stays local)")
print()
print("=== Implementation completeness ===")
print("✓ is_under_root() correctly identifies ROOT and descendants")
print("✓ name.rstrip('/') handles trailing slashes properly")
print("✓ Symlink resolution uses parent directory (correct for relative symlinks)")
print("✓ Hardlink resolution uses target directly (correct for hardlinks)")
VERIFYRepository: manaflow-ai/cmux Length of output: 1671 Ensure symlink and hardlink targets remain within the xcframework tree. Line 37 validates link targets with
Add a check to reject ROOT as a link and verify that resolved link targets remain under 🛡️ Proposed hardening def is_safe_member(name: str) -> bool:
path = PurePosixPath(name)
return not path.is_absolute() and ".." not in path.parts
+def is_under_root(path: PurePosixPath) -> bool:
+ value = str(path)
+ return value == ROOT or value.startswith(ROOT + "/")
+
+
def main() -> None:
@@
if name == ROOT or name == ROOT + "/":
saw_root = True
if member.islnk() or member.issym():
target = normalize(member.linkname)
if not target or not is_safe_member(target):
raise SystemExit(f"unsafe archive link target: {member.linkname}")
+ if name.rstrip("/") == ROOT:
+ raise SystemExit(f"archive root must be a directory: {member.name}")
+
+ target_path = PurePosixPath(target)
+ if member.issym():
+ resolved_target = PurePosixPath(name).parent / target_path
+ else:
+ resolved_target = target_path
+
+ if not is_under_root(resolved_target):
+ raise SystemExit(f"archive link target escapes {ROOT}: {member.linkname}")
elif not (member.isfile() or member.isdir()):
raise SystemExit(f"unsupported archive member: {member.name}")🤖 Prompt for AI Agents |
||
| elif not (member.isfile() or member.isdir()): | ||
| raise SystemExit(f"unsupported archive member: {member.name}") | ||
|
|
||
| if not saw_root: | ||
| raise SystemExit(f"archive missing {ROOT}") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set -edisabled in conditional context risks bad pathsMedium Severity
Because
try_fetch_prebuilt_xcframeworkis invoked inside anelifcondition (line 216), bash disablesset -efor the entire function body. Ifmktemp -dfails at line 156,tmp_dirsilently becomes an empty string, causingtmp_tarto resolve to/GhosttyKit.xcframework.tar.gzandtmp_extractto/extract. Subsequent operations likemkdir -pandcurl -othen target the root filesystem. Themktempfailure and cascading errors are all silently swallowed. An explicit guard aftermktemp(e.g. checkingtmp_diris non-empty and a directory) is needed.Additional Locations (1)
scripts/ensure-ghosttykit.sh#L215-L216Reviewed by Cursor Bugbot for commit 6398d65. Configure here.