Skip to content

Commit fb027fa

Browse files
macmadeclaude
macmade
and
claude
committed
fix: Harden configuration downloads — https-only, status check, timeout, hash
Replace Data(contentsOf:) with a URLSession request bounded by an explicit timeout that only accepts 2xx responses, so a slow host can't hang the download and a 404/500 body is never cached. Require the https scheme both when validating a user-entered URL in the configuration sheet and at download time. Store a content hash beside each cached file and verify it when the cache is read, rejecting a tampered or partially written cache rather than feeding it to the formatter. Add URL.configurationURL(from:) and ConfigurationURLError in the Shared layer for the sheet's inline validation, covered by unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c06ab7a commit fb027fa

6 files changed

Lines changed: 252 additions & 8 deletions

File tree

‎Shared/Configuration.swift‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,20 @@ public class Configuration: NSObject, Codable
119119

120120
private func download( url: URL )
121121
{
122-
guard let sha256 = url.sha256,
123-
let data = try? Data( contentsOf: url ),
122+
guard url.scheme?.lowercased() == "https",
123+
let sha256 = url.sha256,
124124
let container = FileManager.sharedContainerURL?.appendingPathComponent( "Configurations" )
125125
else
126126
{
127127
return
128128
}
129129

130+
guard let data = Configuration.fetch( url: url )
131+
else
132+
{
133+
return
134+
}
135+
130136
try? FileManager.default.createDirectory( at: container, withIntermediateDirectories: true )
131137

132138
let coordinator = NSFileCoordinator( filePresenter: nil )
@@ -136,6 +142,51 @@ public class Configuration: NSObject, Codable
136142
{
137143
try? data.write( to: $0 )
138144
}
145+
146+
// Store a content hash alongside the cached file so a tampered or
147+
// partially-written cache can be detected and rejected at read time.
148+
coordinator.coordinate( writingItemAt: container.appendingPathComponent( "\( sha256 ).sha256" ), error: &error )
149+
{
150+
try? Data( data.sha256.utf8 ).write( to: $0 )
151+
}
152+
}
153+
154+
/// Fetches a configuration over HTTPS with an explicit timeout, returning
155+
/// the body only for a 2xx response. Runs synchronously; intended to be
156+
/// called from a background queue.
157+
private static func fetch( url: URL ) -> Data?
158+
{
159+
var request = URLRequest( url: url, timeoutInterval: 30 )
160+
request.httpMethod = "GET"
161+
162+
let semaphore = DispatchSemaphore( value: 0 )
163+
var result: Data?
164+
165+
let task = URLSession.shared.dataTask( with: request )
166+
{
167+
data, response, error in
168+
169+
defer
170+
{
171+
semaphore.signal()
172+
}
173+
174+
guard error == nil,
175+
let http = response as? HTTPURLResponse,
176+
( 200 ..< 300 ).contains( http.statusCode ),
177+
let data = data
178+
else
179+
{
180+
return
181+
}
182+
183+
result = data
184+
}
185+
186+
task.resume()
187+
semaphore.wait()
188+
189+
return result
139190
}
140191

141192
public func withConfigurations( completion: ( ( swiftFormat: URL?, uncrustify: URL?, finished: () -> Void ) ) -> Void, error: () -> Void )
@@ -205,6 +256,18 @@ public class Configuration: NSObject, Codable
205256
return
206257
}
207258

259+
// If a content hash was stored at download time, the bytes we just
260+
// read must match it; otherwise the cache is tampered or partial and
261+
// must not be fed to the formatter.
262+
let hashURL = container.appendingPathComponent( "\( sha256 ).sha256" )
263+
264+
if let expected = try? String( contentsOf: hashURL, encoding: .utf8 ), expected != data.sha256
265+
{
266+
writeError = NSError( domain: "com.xs-labs.XcodeFormat", code: -1, userInfo: [ NSLocalizedDescriptionKey: "The cached configuration failed its integrity check." ] )
267+
268+
return
269+
}
270+
208271
do
209272
{
210273
try FileManager.default.createDirectory( at: copy.deletingLastPathComponent(), withIntermediateDirectories: true )

‎Shared/ConfigurationURLError.swift‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*******************************************************************************
2+
* The MIT License (MIT)
3+
*
4+
* Copyright (c) 2022, Jean-David Gadina - www.xs-labs.com
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the Software), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
******************************************************************************/
24+
25+
import Foundation
26+
27+
/// A problem with a user-entered configuration URL, surfaced inline in the
28+
/// configuration sheet.
29+
public enum ConfigurationURLError: LocalizedError, Equatable
30+
{
31+
/// The string is not a parseable URL.
32+
case malformed
33+
34+
/// The URL is well-formed but does not use the `https` scheme.
35+
case insecure
36+
37+
public var errorDescription: String?
38+
{
39+
switch self
40+
{
41+
case .malformed: return "Please enter a valid URL"
42+
case .insecure: return "Please enter an https URL"
43+
}
44+
}
45+
}

‎Shared/Extensions/URL.swift‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,32 @@ public extension URL
3030
{
3131
self.absoluteString.sha256
3232
}
33+
34+
/// Validates a user-entered configuration URL string.
35+
///
36+
/// - An empty string means "no URL provided" and returns `nil`.
37+
/// - A string that does not parse as a URL throws `.malformed`.
38+
/// - A URL that does not use the `https` scheme throws `.insecure`.
39+
/// - Otherwise the parsed `https` URL is returned.
40+
static func configurationURL( from string: String ) throws -> URL?
41+
{
42+
if string.isEmpty
43+
{
44+
return nil
45+
}
46+
47+
guard let url = URL( string: string )
48+
else
49+
{
50+
throw ConfigurationURLError.malformed
51+
}
52+
53+
guard url.scheme?.lowercased() == "https"
54+
else
55+
{
56+
throw ConfigurationURLError.insecure
57+
}
58+
59+
return url
60+
}
3361
}

‎SharedTests/ConfigurationURLTests.swift‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*******************************************************************************
2+
* The MIT License (MIT)
3+
*
4+
* Copyright (c) 2022, Jean-David Gadina - www.xs-labs.com
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the Software), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
******************************************************************************/
24+
25+
import Foundation
26+
import Testing
27+
28+
@Suite( "URL.configurationURL" )
29+
struct ConfigurationURLTests
30+
{
31+
@Test( "Returns nil for an empty string (no URL provided)" )
32+
func emptyIsNil() throws
33+
{
34+
#expect( try URL.configurationURL( from: "" ) == nil )
35+
}
36+
37+
@Test( "Accepts a well-formed https URL" )
38+
func acceptsHTTPS() throws
39+
{
40+
let url = try URL.configurationURL( from: "https://example.com/config" )
41+
42+
#expect( url == URL( string: "https://example.com/config" ) )
43+
}
44+
45+
@Test( "Rejects an http URL as insecure" )
46+
func rejectsHTTP()
47+
{
48+
#expect( throws: ConfigurationURLError.insecure )
49+
{
50+
try URL.configurationURL( from: "http://example.com/config" )
51+
}
52+
}
53+
54+
@Test( "Rejects a non-https scheme as insecure" )
55+
func rejectsOtherSchemes()
56+
{
57+
#expect( throws: ConfigurationURLError.insecure )
58+
{
59+
try URL.configurationURL( from: "ftp://example.com/config" )
60+
}
61+
}
62+
63+
@Test( "Rejects a URL with no scheme as insecure" )
64+
func rejectsSchemeless()
65+
{
66+
#expect( throws: ConfigurationURLError.insecure )
67+
{
68+
try URL.configurationURL( from: "example.com/config" )
69+
}
70+
}
71+
72+
@Test( "Rejects a malformed URL string" )
73+
func rejectsMalformed()
74+
{
75+
#expect( throws: ConfigurationURLError.malformed )
76+
{
77+
try URL.configurationURL( from: "https://exa mple.com/has spaces" )
78+
}
79+
}
80+
81+
@Test( "Error messages are user-facing and distinct" )
82+
func errorMessages()
83+
{
84+
#expect( ConfigurationURLError.malformed.errorDescription?.isEmpty == false )
85+
#expect( ConfigurationURLError.insecure.errorDescription?.isEmpty == false )
86+
#expect( ConfigurationURLError.malformed.errorDescription != ConfigurationURLError.insecure.errorDescription )
87+
}
88+
}

‎XcodeFormat.xcodeproj/project.pbxproj‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
05BED21B28EC4A2A008039F6 /* Task.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05BED21928EC4A2A008039F6 /* Task.swift */; };
6161
05C2595E28EC8233008BDDDE /* XcodeFormat.workflow in Resources */ = {isa = PBXBuildFile; fileRef = 05C2595B28EC8233008BDDDE /* XcodeFormat.workflow */; };
6262
1FC94E1B2C6A97C099672427 /* Cocoa.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 754426165B1A6F7B9A8AA57D /* Cocoa.framework */; };
63+
22A6089A621DED5B56E879A7 /* ConfigurationURLError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2EF3AB7428EF979DDF19D7B6 /* ConfigurationURLError.swift */; };
6364
393AAC40E0670389A6DA1F01 /* FormatterOutcome.swift in Sources */ = {isa = PBXBuildFile; fileRef = B525D96BC0786913A529715D /* FormatterOutcome.swift */; };
6465
3D5B82DA28BD1E95AA93253A /* CursorPositionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4D7074E40EFC210EB5BEF6A5 /* CursorPositionTests.swift */; };
6566
415B69E897EF098F1D497C6B /* String.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05744ED528EC2EDB00A88503 /* String.swift */; };
@@ -72,13 +73,16 @@
7273
5D1B0EB0A339B282FE2884BB /* FormatterOutcome.swift in Sources */ = {isa = PBXBuildFile; fileRef = B525D96BC0786913A529715D /* FormatterOutcome.swift */; };
7374
6AF1DB0943F9AA065EA80860 /* URLTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B907F7DC445EA71845A03EE9 /* URLTests.swift */; };
7475
6CE2C56F18102DBE76DAA3AD /* UncrustifyLanguageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9872753262CE5731D808C3E0 /* UncrustifyLanguageTests.swift */; };
76+
70BB8D962A5472EC9F16822D /* ConfigurationURLError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2EF3AB7428EF979DDF19D7B6 /* ConfigurationURLError.swift */; };
7577
757F7D103067E09A06177437 /* UncrustifyLanguage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 156C3284ADF849DBE528A272 /* UncrustifyLanguage.swift */; };
7678
7CA215582B396CDC75D0E7A5 /* Data.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05744ED228EC2ED400A88503 /* Data.swift */; };
7779
8D26803DA75870DC7226B81F /* CursorPosition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B57CE4A604090770D7E613D /* CursorPosition.swift */; };
7880
8E40D02C91DF78B1D74D6200 /* CursorPosition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B57CE4A604090770D7E613D /* CursorPosition.swift */; };
81+
A1A51DCE488C2DE67F580ABE /* ConfigurationURLError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2EF3AB7428EF979DDF19D7B6 /* ConfigurationURLError.swift */; };
7982
B9C0864239244AC3DB7628F7 /* FormatterError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BB18C081049DED53951BA35 /* FormatterError.swift */; };
8083
C9086B5A19ACECE6A1E9E4D9 /* FormatterError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BB18C081049DED53951BA35 /* FormatterError.swift */; };
8184
CA9C26A026E8351452F19284 /* UncrustifyLanguage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 156C3284ADF849DBE528A272 /* UncrustifyLanguage.swift */; };
85+
E0AECB558C4E544DCCDCA029 /* ConfigurationURLTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 063A2606544CBB8F999B2511 /* ConfigurationURLTests.swift */; };
8286
E1F98905A928280130247424 /* UncrustifyLanguage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 156C3284ADF849DBE528A272 /* UncrustifyLanguage.swift */; };
8387
EF9CF118DCDD7951D6C90043 /* Configuration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 050E0F6128EB66390080C562 /* Configuration.swift */; };
8488
F05F93196B0323DB94F21A1A /* CursorPosition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B57CE4A604090770D7E613D /* CursorPosition.swift */; };
@@ -255,8 +259,10 @@
255259
0594D0002FE1203B002650F9 /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.xib; name = Base; path = Base.lproj/ConfigurationWindowController.xib; sourceTree = "<group>"; };
256260
05BED21928EC4A2A008039F6 /* Task.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Task.swift; sourceTree = "<group>"; };
257261
05C2595B28EC8233008BDDDE /* XcodeFormat.workflow */ = {isa = PBXFileReference; lastKnownFileType = wrapper; path = XcodeFormat.workflow; sourceTree = "<group>"; };
262+
063A2606544CBB8F999B2511 /* ConfigurationURLTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ConfigurationURLTests.swift; sourceTree = "<group>"; };
258263
156C3284ADF849DBE528A272 /* UncrustifyLanguage.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = UncrustifyLanguage.swift; sourceTree = "<group>"; };
259264
2B7221495008B0E46F3003F1 /* SharedTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = SharedTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
265+
2EF3AB7428EF979DDF19D7B6 /* ConfigurationURLError.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ConfigurationURLError.swift; sourceTree = "<group>"; };
260266
3B49163E30C3B9E7BC97B7C6 /* FormatterErrorTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = FormatterErrorTests.swift; sourceTree = "<group>"; };
261267
3B57CE4A604090770D7E613D /* CursorPosition.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = CursorPosition.swift; sourceTree = "<group>"; };
262268
3BB18C081049DED53951BA35 /* FormatterError.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = FormatterError.swift; sourceTree = "<group>"; };
@@ -564,6 +570,7 @@
564570
B525D96BC0786913A529715D /* FormatterOutcome.swift */,
565571
3BB18C081049DED53951BA35 /* FormatterError.swift */,
566572
3B57CE4A604090770D7E613D /* CursorPosition.swift */,
573+
2EF3AB7428EF979DDF19D7B6 /* ConfigurationURLError.swift */,
567574
);
568575
path = Shared;
569576
sourceTree = "<group>";
@@ -606,6 +613,7 @@
606613
D8EAF9AF3502DEDA70432E27 /* FormatterOutcomeTests.swift */,
607614
3B49163E30C3B9E7BC97B7C6 /* FormatterErrorTests.swift */,
608615
4D7074E40EFC210EB5BEF6A5 /* CursorPositionTests.swift */,
616+
063A2606544CBB8F999B2511 /* ConfigurationURLTests.swift */,
609617
);
610618
name = SharedTests;
611619
path = SharedTests;
@@ -820,6 +828,7 @@
820828
393AAC40E0670389A6DA1F01 /* FormatterOutcome.swift in Sources */,
821829
B9C0864239244AC3DB7628F7 /* FormatterError.swift in Sources */,
822830
F05F93196B0323DB94F21A1A /* CursorPosition.swift in Sources */,
831+
A1A51DCE488C2DE67F580ABE /* ConfigurationURLError.swift in Sources */,
823832
);
824833
runOnlyForDeploymentPostprocessing = 0;
825834
};
@@ -840,6 +849,7 @@
840849
FFFFB23BFC620B462FF4B4DB /* FormatterOutcome.swift in Sources */,
841850
520812B973DDE22F15845E54 /* FormatterError.swift in Sources */,
842851
8D26803DA75870DC7226B81F /* CursorPosition.swift in Sources */,
852+
22A6089A621DED5B56E879A7 /* ConfigurationURLError.swift in Sources */,
843853
);
844854
runOnlyForDeploymentPostprocessing = 0;
845855
};
@@ -864,6 +874,8 @@
864874
430C2F21B01A0C20B2A0B41D /* FormatterErrorTests.swift in Sources */,
865875
3D5B82DA28BD1E95AA93253A /* CursorPositionTests.swift in Sources */,
866876
8E40D02C91DF78B1D74D6200 /* CursorPosition.swift in Sources */,
877+
E0AECB558C4E544DCCDCA029 /* ConfigurationURLTests.swift in Sources */,
878+
70BB8D962A5472EC9F16822D /* ConfigurationURLError.swift in Sources */,
867879
);
868880
runOnlyForDeploymentPostprocessing = 0;
869881
};

‎XcodeFormat/Classes/ConfigurationWindowController.swift‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,27 @@ public class ConfigurationWindowController: NSWindowController
105105
return
106106
}
107107

108-
let swiftFormat = URL( string: self.swiftFormat )
109-
let uncrustify = URL( string: self.uncrustify )
108+
let swiftFormat: URL?
109+
let uncrustify: URL?
110110

111-
if self.swiftFormat.isEmpty == false, swiftFormat == nil
111+
do
112112
{
113-
self.swiftFormatError = "Please enter a valid URL"
113+
swiftFormat = try URL.configurationURL( from: self.swiftFormat )
114+
}
115+
catch
116+
{
117+
self.swiftFormatError = error.localizedDescription
114118

115119
return
116120
}
117121

118-
if self.uncrustify.isEmpty == false, uncrustify == nil
122+
do
123+
{
124+
uncrustify = try URL.configurationURL( from: self.uncrustify )
125+
}
126+
catch
119127
{
120-
self.uncrustifyError = "Please enter a valid URL"
128+
self.uncrustifyError = error.localizedDescription
121129

122130
return
123131
}

0 commit comments

Comments
 (0)