Skip to content

Commit a40ea5b

Browse files
p-linnaneclaude
andauthored
fix: hardening pass from a fresh review (scanner, CI, app, site, docs) (#491)
Untrusted-binary safety in the scanner, supply-chain hardening in the release/scan workflows, app state-management fixes, SSR security headers, CLI robustness, and README/CLAUDE.md accuracy. All checks green; 187 tests pass under TSan/ASan. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 81b2e80 commit a40ea5b

21 files changed

Lines changed: 635 additions & 312 deletions

.github/workflows/release.yml

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,20 @@ jobs:
7373
- name: Create temporary certificate file
7474
env:
7575
DEVELOPER_ID_CERTIFICATE: ${{ secrets.DEVELOPER_ID_CERTIFICATE }}
76-
run: echo -n "${DEVELOPER_ID_CERTIFICATE}" |
77-
base64 --decode --output="${RUNNER_TEMP}/${TEMPORARY_CERTIFICATE_FILE}"
76+
run: |
77+
set +x # don't xtrace the base64-encoded certificate
78+
echo -n "${DEVELOPER_ID_CERTIFICATE}" |
79+
base64 --decode --output="${RUNNER_TEMP}/${TEMPORARY_CERTIFICATE_FILE}"
7880
7981
- name: Import certificate into keychain
8082
env:
8183
DEVELOPER_ID_CERTIFICATE_PASSWORD: ${{ secrets.DEVELOPER_ID_CERTIFICATE_PASSWORD }}
82-
run: security import "${RUNNER_TEMP}/${TEMPORARY_CERTIFICATE_FILE}"
83-
-k "${RUNNER_TEMP}/${TEMPORARY_KEYCHAIN_FILE}"
84-
-P "${DEVELOPER_ID_CERTIFICATE_PASSWORD}"
85-
-t cert -f pkcs12 -A
84+
run: |
85+
set +x # don't xtrace the certificate password
86+
security import "${RUNNER_TEMP}/${TEMPORARY_CERTIFICATE_FILE}" \
87+
-k "${RUNNER_TEMP}/${TEMPORARY_KEYCHAIN_FILE}" \
88+
-P "${DEVELOPER_ID_CERTIFICATE_PASSWORD}" \
89+
-t cert -f pkcs12 -A
8690
8791
- name: Clean up temporary certificate file
8892
if: always()
@@ -173,6 +177,7 @@ jobs:
173177
NOTARIZATION_PASSWORD: ${{ secrets.NOTARIZATION_PASSWORD }}
174178
ARTIFACT_NAME: ${{ needs.build.outputs.artifact_name }}
175179
run: |
180+
set +x # keep the notarization password out of the xtrace log
176181
xcrun notarytool submit "${ARTIFACT_NAME}" \
177182
--team-id "${DEVELOPMENT_TEAM}" \
178183
--apple-id "${APPLE_ID}" \
@@ -185,9 +190,15 @@ jobs:
185190
"${ARTIFACT_NAME}"
186191
187192
- name: Download Sparkle
193+
env:
194+
# Keep in lockstep with the Sparkle framework version in Package.resolved
195+
# so the sign_update tool matches the runtime that verifies updates.
196+
SPARKLE_VERSION: "2.9.2"
197+
SPARKLE_SHA256: "1cb340cbbef04c6c0d162078610c25e2221031d794a3449d89f2f56f4df77c95"
188198
run: |
189-
curl -L -o "${RUNNER_TEMP}/sparkle.tar.xz" \
190-
"https://github.com/sparkle-project/Sparkle/releases/download/2.8.1/Sparkle-2.8.1.tar.xz"
199+
curl -fL -o "${RUNNER_TEMP}/sparkle.tar.xz" \
200+
"https://github.com/sparkle-project/Sparkle/releases/download/${SPARKLE_VERSION}/Sparkle-${SPARKLE_VERSION}.tar.xz"
201+
echo "${SPARKLE_SHA256} ${RUNNER_TEMP}/sparkle.tar.xz" | shasum -a 256 -c -
191202
mkdir -p "${RUNNER_TEMP}/sparkle"
192203
tar xf "${RUNNER_TEMP}/sparkle.tar.xz" -C "${RUNNER_TEMP}/sparkle"
193204
@@ -196,6 +207,7 @@ jobs:
196207
SPARKLE_KEY: ${{ secrets.SPARKLE_PRIVATE_KEY }}
197208
ARTIFACT_NAME: ${{ needs.build.outputs.artifact_name }}
198209
run: |
210+
set +x # never xtrace the EdDSA private key
199211
SPARKLE_KEY_FILE="${RUNNER_TEMP}/sparkle_private_key"
200212
echo "${SPARKLE_KEY}" > "${SPARKLE_KEY_FILE}"
201213
SIG_OUTPUT=$("${RUNNER_TEMP}/sparkle/bin/sign_update" "${ARTIFACT_NAME}" --ed-key-file "${SPARKLE_KEY_FILE}" 2>&1)

.github/workflows/scan-xip.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ jobs:
101101
BUILD: ${{ inputs.build_number }}
102102
run: |
103103
XIP_PATH=$(echo "${XIP_URL}" | sed -E 's/.*[?&]path=([^&]*).*/\1/')
104+
# The ADCDownloadAuth secret cookie is sent to adcdownload.apple.com${XIP_PATH}.
105+
# Reject anything that isn't a clean absolute .xip path so a value like
106+
# "@evil.example.com/x.xip" can't redirect curl (and the cookie) to another host.
107+
if [[ ! "${XIP_PATH}" =~ ^/[A-Za-z0-9._~%/+-]+\.xip$ ]]; then
108+
echo "::error::Refusing to use download path '${XIP_PATH}': not a clean absolute .xip path."
109+
exit 1
110+
fi
104111
ORIG_NAME=$(basename "${XIP_PATH}")
105112
VERSION=$(echo "${ORIG_NAME}" | sed -E 's/Xcode_([0-9]+(\.[0-9]+)*).*/\1/')
106113
MAJOR=${VERSION%%.*}

CLAUDE.md

Lines changed: 132 additions & 135 deletions
Large diffs are not rendered by default.

README.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ macOSdb scans Apple's IPSW firmware files and Xcode `.xip` archives, extracts ve
1212

1313
## Tracked components
1414

15-
**Filesystem binaries:** curl, httpd, LibreSSL, OpenSSH, Ruby, SQLite, vim, zsh
15+
**Filesystem binaries:** curl, httpd, LibreSSL, OpenSSH, Ruby, sudo, SQLite, vim, zsh
1616

1717
**Dyld shared cache:** libbz2, libcurl, libexpat, libncurses, libpcap, libsqlite3, libssl, libxml2
1818

19-
**Xcode toolchain:** Apple Clang, cctools, Git, ld, lldb, Swift
19+
**Xcode toolchain:** Apple Clang, cctools, Git, ld, lldb, Python, Swift
2020

21-
**SDK libraries:** bzip2, expat, libcurl, libedit, libexslt, libffi, libxml2, libxslt, ncurses, sqlite3, zlib
21+
**SDK libraries:** bzip2, expat, libcurl, libexslt, libffi, libxml2, libxslt, ncurses, sqlite3, zlib
2222

2323
## Installation
2424

@@ -73,9 +73,13 @@ macosdb scan ~/Downloads/UniversalMac_15.2_24C101_Restore.ipsw \
7373
macosdb scan ~/Downloads/Xcode_26.4_Apple_silicon.xip \
7474
--output data/xcode/releases --release-date 2026-03-24 --update-index --verbose
7575

76-
# Validate archives and create SHA-256 sidecar hashes
76+
# Validate archives: create SHA-256 sidecars, or verify against existing ones
7777
macosdb validate ~/Downloads/UniversalMac_15.2_24C101_Restore.ipsw
7878
macosdb validate --dir /path/to/archive
79+
80+
# Clean up leftover mounted DMGs and temp directories from aborted scans
81+
macosdb cleanup # dry run — list what would be removed
82+
macosdb cleanup --force # actually unmount and delete
7983
```
8084

8185
### Shell completions
@@ -135,10 +139,12 @@ just test # Run Swift tests
135139
just lint # Run SwiftLint (--strict)
136140
just lint-json # Validate JSON data files
137141
just typos # Check for typos
138-
just audit # Audit GitHub Actions workflows
142+
just audit # Audit GitHub Actions workflows (zizmor)
143+
just periphery # Scan for unused code
139144
just build-app # Build the app with xcodebuild
140145
just test-xcode # Run tests with xcodebuild (matches CI)
141-
just check # Run all checks (lint, lint-json, test, audit, site format, site build)
146+
just lychee # Check the built site for broken links
147+
just check # Run all checks (lint, lint-json, typos, audit, periphery, test, site format, site build)
142148
```
143149

144150
### Site

Sources/macOSdbKit/DataProvider.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,19 @@ public actor DataProvider {
4444
}
4545

4646
public func fetchRelease(_ entry: ReleaseIndexEntry) async throws -> Release {
47-
if let cached = cachedReleases[entry.buildNumber] {
47+
// Build numbers aren't guaranteed unique across products, so namespace the
48+
// cache key by product directory.
49+
let cacheKey = "\(entry.resolvedProductType.dataDirectory)/\(entry.buildNumber)"
50+
if let cached = cachedReleases[cacheKey] {
4851
return cached
4952
}
5053

54+
// dataFile comes from the index; reject path traversal so a crafted index
55+
// can't read outside the data directory when baseURL is a local file:// path.
56+
guard !entry.dataFile.contains("..") else {
57+
throw DataProviderError.invalidDataFile(entry.dataFile)
58+
}
59+
5160
let relativePath = "\(entry.resolvedProductType.dataDirectory)/\(entry.dataFile)"
5261
let url = baseURL.appendingPathComponent(relativePath)
5362
Self.logger.debug("Fetching release data from \(url)")
@@ -57,7 +66,7 @@ public actor DataProvider {
5766

5867
let decoder = JSONDecoder()
5968
let release = try decoder.decode(Release.self, from: data)
60-
cachedReleases[entry.buildNumber] = release
69+
cachedReleases[cacheKey] = release
6170

6271
Self.logger.info("Loaded release: \(release.displayName) (\(release.buildNumber))")
6372
return release
@@ -125,13 +134,16 @@ public actor DataProvider {
125134
enum DataProviderError: LocalizedError {
126135
case httpError(statusCode: Int, url: URL)
127136
case allReleasesFailed(count: Int)
137+
case invalidDataFile(String)
128138

129139
var errorDescription: String? {
130140
switch self {
131141
case .httpError(let statusCode, let url):
132142
"HTTP \(statusCode) fetching \(url)"
133143
case .allReleasesFailed(let count):
134144
"Loaded the release index but failed to fetch any of its \(count) releases"
145+
case .invalidDataFile(let path):
146+
"Refusing to fetch release data with an unsafe path: \(path)"
135147
}
136148
}
137149
}

Sources/macOSdbKit/Scanner/AEADecryptor.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ actor AEADecryptor {
131131

132132
private func validateHeader(_ header: Data) throws -> Int {
133133
let bytes = [UInt8](header)
134+
guard bytes.count >= 12 else {
135+
throw ScannerError.aeaDecryptionFailed(reason: "AEA header too short")
136+
}
134137

135138
guard bytes[0] == 0x41, bytes[1] == 0x45,
136139
bytes[2] == 0x41, bytes[3] == 0x31 else {

Sources/macOSdbKit/Scanner/DMGMounter.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,18 @@ actor DMGMounter {
2424
process.standardError = stderr
2525

2626
try process.run()
27+
// Drain both pipes before waiting: a large -plist output on stdout could
28+
// otherwise fill the pipe buffer and deadlock against waitUntilExit().
29+
let outputData = stdout.fileHandleForReading.readDataToEndOfFile()
30+
let errorData = stderr.fileHandleForReading.readDataToEndOfFile()
2731
process.waitUntilExit()
2832

2933
guard process.terminationStatus == 0 else {
30-
let errorData = stderr.fileHandleForReading.readDataToEndOfFile()
3134
let errorMessage = String(data: errorData, encoding: .utf8) ?? "unknown error"
3235
Self.logger.error("hdiutil attach failed: \(errorMessage)")
3336
throw ScannerError.dmgMountFailed(path: dmgPath.path, reason: errorMessage)
3437
}
3538

36-
let outputData = stdout.fileHandleForReading.readDataToEndOfFile()
3739
return try parseMountOutput(outputData, dmgPath: dmgPath.path)
3840
}
3941

@@ -50,10 +52,10 @@ actor DMGMounter {
5052

5153
do {
5254
try process.run()
55+
let errorData = stderr.fileHandleForReading.readDataToEndOfFile()
5356
process.waitUntilExit()
5457

5558
if process.terminationStatus != 0 {
56-
let errorData = stderr.fileHandleForReading.readDataToEndOfFile()
5759
let errorMessage = String(data: errorData, encoding: .utf8) ?? "unknown error"
5860
Self.logger.warning("hdiutil detach warning: \(errorMessage)")
5961
}

0 commit comments

Comments
 (0)