From 98c6a1753738cdc846daf162348f5a22507c9ee2 Mon Sep 17 00:00:00 2001 From: cweinberger Date: Wed, 26 Feb 2020 13:15:20 +0100 Subject: [PATCH 1/3] Add tests for the keyFilter feature & apply filter to headers and url params as well. --- Sources/Bugsnag/Bugsnag.swift | 22 +++- Tests/BugsnagTests/BugsnagTests.swift | 168 ++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 2 deletions(-) diff --git a/Sources/Bugsnag/Bugsnag.swift b/Sources/Bugsnag/Bugsnag.swift index 0f19e2d..bfc01a2 100644 --- a/Sources/Bugsnag/Bugsnag.swift +++ b/Sources/Bugsnag/Bugsnag.swift @@ -112,10 +112,11 @@ struct BugsnagRequest: Encodable { init(httpRequest: HTTPRequest, keyFilters: [String]) { self.body = BugsnagRequest.filter(httpRequest.body, using: keyFilters) self.clientIp = httpRequest.remotePeer.hostname - self.headers = Dictionary(httpRequest.headers.map { $0 }) { first, second in second } + let filteredHeaders = BugsnagRequest.filter(httpRequest.headers, using: keyFilters) + self.headers = Dictionary(filteredHeaders.map { $0 }) { first, second in second } self.httpMethod = httpRequest.method.string self.referer = httpRequest.remotePeer.description - self.url = httpRequest.urlString + self.url = BugsnagRequest.filter(httpRequest.urlString, using: keyFilters) } static private func filter(_ body: HTTPBody, using filters: [String]) -> String? { @@ -131,6 +132,23 @@ struct BugsnagRequest: Encodable { let json = try? JSONSerialization.data(withJSONObject: filtered, options: [.prettyPrinted]) return json.flatMap { String(data: $0, encoding: .utf8) } } + + static private func filter(_ headers: HTTPHeaders, using filters: [String]) -> HTTPHeaders { + var mutableHeaders = headers + filters.forEach { mutableHeaders.remove(name: $0) } + return mutableHeaders + } + + /** + @discussion Currently returns the original (unfiltered) url if anything goes wrong. + */ + static private func filter(_ urlString: String, using filters: [String]) -> String { + guard var urlComponents = URLComponents(string: urlString) else { + return urlString + } + urlComponents.queryItems?.removeAll(where: { filters.contains($0.name) }) + return urlComponents.string ?? urlString + } } struct BugsnagThread: Encodable { diff --git a/Tests/BugsnagTests/BugsnagTests.swift b/Tests/BugsnagTests/BugsnagTests.swift index 1b1a891..9dbffaa 100644 --- a/Tests/BugsnagTests/BugsnagTests.swift +++ b/Tests/BugsnagTests/BugsnagTests.swift @@ -55,4 +55,172 @@ final class BugsnagTests: XCTestCase { let request = Request(using: application) try reporter.report(NotFound(), on: request).wait() } + + func testKeyFiltersWorkInRequestBody() throws { + var capturedSendReportParameters: ( + host: String, + headers: HTTPHeaders, + body: Data, + container: Container + )? + + let reporter = BugsnagReporter( + config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]), + sendReport: { host, headers, data, container in + capturedSendReportParameters = (host, headers, data, container) + return container.future(Response(http: HTTPResponse(status: .ok), using: container)) + }) + let application = try Application.test() + let request = Request(using: application) + request.http.method = .POST + request.http.body = TestBody.default.httpBody + + _ = try! reporter.report(NotFound(), on: request).wait() + + guard let params = capturedSendReportParameters else { + XCTFail() + return + } + + let responseBody = try JSONDecoder().decode(BugsnagResponseBody.self, from: params.body) + + guard let body = responseBody.events.first?.request?.body else { + XCTFail("Unable to parse request body") + return + } + XCTAssertNil(body.password, "test that password is removed") + XCTAssertNil(body.email, "test that email is removed") + XCTAssertEqual(body.hash, TestBody.default.hash, "test that hash is not altered") + } + + func testKeyFiltersWorkInHeaderFields() throws { + var capturedSendReportParameters: ( + host: String, + headers: HTTPHeaders, + body: Data, + container: Container + )? + + let reporter = BugsnagReporter( + config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]), + sendReport: { host, headers, data, container in + capturedSendReportParameters = (host, headers, data, container) + return container.future(Response(http: HTTPResponse(status: .ok), using: container)) + }) + let application = try Application.test() + let request = Request(using: application) + request.http.method = .POST + request.http.body = TestBody.default.httpBody + var headers = request.http.headers + headers.add(name: HTTPHeaderName("password"), value: TestBody.default.password!) + headers.add(name: HTTPHeaderName("email"), value: TestBody.default.email!) + headers.add(name: HTTPHeaderName("hash"), value: TestBody.default.hash!) + request.http.headers = headers + + _ = try! reporter.report(NotFound(), on: request).wait() + + guard let params = capturedSendReportParameters else { + XCTFail() + return + } + + let responseBody = try JSONDecoder().decode(BugsnagResponseBody.self, from: params.body) + + guard let responseHeaders = responseBody.events.first?.request?.headers else { + XCTFail("Unable to parse response headers") + return + } + + XCTAssertNil(responseHeaders["password"], "test that password is removed") + XCTAssertNil(responseHeaders["email"], "test that email is removed") + XCTAssertEqual(responseHeaders["hash"], TestBody.default.hash!, "test that hash is not altered") + } + + func testKeyFiltersWorkInURLQueryParams() throws { + var capturedSendReportParameters: ( + host: String, + headers: HTTPHeaders, + body: Data, + container: Container + )? + + let reporter = BugsnagReporter( + config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]), + sendReport: { host, headers, data, container in + capturedSendReportParameters = (host, headers, data, container) + return container.future(Response(http: HTTPResponse(status: .ok), using: container)) + }) + let application = try Application.test() + let request = Request(using: application) + request.http.url = URL(string: "http://foo.bar.com/?password=\(TestBody.default.password!)&email=\(TestBody.default.email!)&hash=\(TestBody.default.hash!)")! + request.http.method = .POST + request.http.body = TestBody.default.httpBody + var headers = request.http.headers + headers.add(name: HTTPHeaderName("password"), value: TestBody.default.password!) + headers.add(name: HTTPHeaderName("email"), value: TestBody.default.email!) + headers.add(name: HTTPHeaderName("hash"), value: TestBody.default.hash!) + request.http.headers = headers + + _ = try! reporter.report(NotFound(), on: request).wait() + + guard let params = capturedSendReportParameters else { + XCTFail() + return + } + + let responseBody = try JSONDecoder().decode(BugsnagResponseBody.self, from: params.body) + + guard let responseURLString = responseBody.events.first?.request?.url else { + XCTFail("Unable to parse response url") + return + } + + let urlComponents = URLComponents(string: responseURLString) + let passwordItem = urlComponents?.queryItems?.filter { $0.name == "password" }.last + let emailItem = urlComponents?.queryItems?.filter { $0.name == "email" }.last + let hashItem = urlComponents?.queryItems?.filter { $0.name == "hash" }.last + + XCTAssertNil(passwordItem, "test that password is removed") + XCTAssertNil(emailItem, "test that email is removed") + XCTAssertEqual(hashItem?.value, TestBody.default.hash!, "test that hash is not altered") + } +} + +struct TestBody: Codable { + var password: String? + var email: String? + var hash: String? + + static var `default`: Self { + return .init(password: "TopSecret", email: "foo@bar.com", hash: "myAwesomeHash") + } + + var httpBody: HTTPBody { + return try! HTTPBody(data: JSONEncoder().encode(self)) + } +} + +struct BugsnagResponseBody: Codable { + struct Event: Codable { + struct Request: Codable { + let body: T? + let headers: [String: String]? + let url: String? + + // custom decoding needed as the format is JSON string (not JSON object) + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let bodyString = try container.decode(String.self, forKey: .body) + guard let data = bodyString.data(using: .utf8) else { + throw Abort(.internalServerError) + } + body = try JSONDecoder().decode(T.self, from: data) + headers = try container.decode(Dictionary.self, forKey: .headers) + url = try container.decode(String.self, forKey: .url) + } + } + let request: Request? + } + let apiKey: String + let events: [Event] } From d181326c02f18b5bfa54c3181e58bf95f624ed05 Mon Sep 17 00:00:00 2001 From: cweinberger Date: Wed, 26 Feb 2020 16:32:41 +0100 Subject: [PATCH 2/3] Use Set for keyFilter internally & generate linux main --- Sources/Bugsnag/Bugsnag.swift | 10 +++++----- Sources/Bugsnag/BugsnagConfig.swift | 4 ++-- Tests/BugsnagTests/XCTestManifests.swift | 3 +++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Sources/Bugsnag/Bugsnag.swift b/Sources/Bugsnag/Bugsnag.swift index bfc01a2..3c18524 100644 --- a/Sources/Bugsnag/Bugsnag.swift +++ b/Sources/Bugsnag/Bugsnag.swift @@ -33,7 +33,7 @@ struct BugsnagEvent: Encodable { breadcrumbs: [BugsnagBreadcrumb], error: Error, httpRequest: HTTPRequest? = nil, - keyFilters: [String], + keyFilters: Set, metadata: [String: CustomDebugStringConvertible], payloadVersion: String, severity: Severity, @@ -109,7 +109,7 @@ struct BugsnagRequest: Encodable { let referer: String let url: String - init(httpRequest: HTTPRequest, keyFilters: [String]) { + init(httpRequest: HTTPRequest, keyFilters: Set) { self.body = BugsnagRequest.filter(httpRequest.body, using: keyFilters) self.clientIp = httpRequest.remotePeer.hostname let filteredHeaders = BugsnagRequest.filter(httpRequest.headers, using: keyFilters) @@ -119,7 +119,7 @@ struct BugsnagRequest: Encodable { self.url = BugsnagRequest.filter(httpRequest.urlString, using: keyFilters) } - static private func filter(_ body: HTTPBody, using filters: [String]) -> String? { + static private func filter(_ body: HTTPBody, using filters: Set) -> String? { guard let data = body.data, let unwrap = try? JSONSerialization.jsonObject(with: data) as? [String: Any], @@ -133,7 +133,7 @@ struct BugsnagRequest: Encodable { return json.flatMap { String(data: $0, encoding: .utf8) } } - static private func filter(_ headers: HTTPHeaders, using filters: [String]) -> HTTPHeaders { + static private func filter(_ headers: HTTPHeaders, using filters: Set) -> HTTPHeaders { var mutableHeaders = headers filters.forEach { mutableHeaders.remove(name: $0) } return mutableHeaders @@ -142,7 +142,7 @@ struct BugsnagRequest: Encodable { /** @discussion Currently returns the original (unfiltered) url if anything goes wrong. */ - static private func filter(_ urlString: String, using filters: [String]) -> String { + static private func filter(_ urlString: String, using filters: Set) -> String { guard var urlComponents = URLComponents(string: urlString) else { return urlString } diff --git a/Sources/Bugsnag/BugsnagConfig.swift b/Sources/Bugsnag/BugsnagConfig.swift index a3d21fb..5400b22 100644 --- a/Sources/Bugsnag/BugsnagConfig.swift +++ b/Sources/Bugsnag/BugsnagConfig.swift @@ -3,7 +3,7 @@ public struct BugsnagConfig { let releaseStage: String /// A version identifier, (eg. a git hash) let version: String? - let keyFilters: [String] + let keyFilters: Set let shouldReport: Bool let debug: Bool @@ -18,7 +18,7 @@ public struct BugsnagConfig { self.apiKey = apiKey self.releaseStage = releaseStage self.version = version - self.keyFilters = keyFilters + self.keyFilters = Set(keyFilters) self.shouldReport = shouldReport self.debug = debug } diff --git a/Tests/BugsnagTests/XCTestManifests.swift b/Tests/BugsnagTests/XCTestManifests.swift index 06b4115..2a96233 100644 --- a/Tests/BugsnagTests/XCTestManifests.swift +++ b/Tests/BugsnagTests/XCTestManifests.swift @@ -6,6 +6,9 @@ extension BugsnagTests { // `swift test --generate-linuxmain` // to regenerate. static let __allTests__BugsnagTests = [ + ("testKeyFiltersWorkInHeaderFields", testKeyFiltersWorkInHeaderFields), + ("testKeyFiltersWorkInRequestBody", testKeyFiltersWorkInRequestBody), + ("testKeyFiltersWorkInURLQueryParams", testKeyFiltersWorkInURLQueryParams), ("testReportingCanBeDisabled", testReportingCanBeDisabled), ("testSendReport", testSendReport), ] From d2b5c47701f4742ed0d684cf23357fe5d80e6a92 Mon Sep 17 00:00:00 2001 From: cweinberger Date: Wed, 26 Feb 2020 16:39:13 +0100 Subject: [PATCH 3/3] Add README for keyFilters feature --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index 5aec084..0479076 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,20 @@ enum BreadcrumbType { } ``` +#### Filter out fields from reports +Usually you will receive information such as headers, query params or post body fields in the reports from Bugsnag. To ensure that you do not track sensitive information, you can configure Bugsnag with a list of fields that should be filtered out: + +```swift +BugsnagConfig( + apiKey: "apiKey", + releaseStage: "test", + keyFilters: ["password", "email", "authorization", "lastname"] +) +``` +In this case Bugsnag Reports won't contain header fields, query params or post body json fields with the keys/names **password**, **email**, **authorization**, **lastname**. + +⚠️ Note: in JSON bodies, this only works for the first level of fields and not for nested children. + ## 🏆 Credits This package is developed and maintained by the Vapor team at [Nodes](https://www.nodesagency.com).