- Added a shared TrustValidation.extractServerTrust helper to central…

[tkausch]

User description

• CertificateCAPinning and PublicKeyServerPinning now reuse it and emit consistent success/failure messages.
• Renamed BearerReqeustAuthorizer to BearerRequestAuthorizer, added optional logger injection to both Basic and Bearer authorizers, and updated their logging internals

---

PR Type

Enhancement, Bug fix

---

Description

• Centralized SecTrust extraction logic into new TrustValidation.extractServerTrust helper

• Unified error messages and logging across pinning delegates for consistency

• Refactored pinning validation logic using guard statements for clarity

• Added optional logger injection to Basic and Bearer authorizers

• Fixed typo: renamed BearerReqeustAuthorizer to BearerRequestAuthorizer

---

Diagram Walkthrough

flowchart LR
  A["TrustValidation<br/>extractServerTrust"] --> B["CertificateCAPinning"]
  A --> C["PublicKeyServerPinning"]
  D["BasicRequestAuthorizer<br/>optional logger"] --> E["URLRequestAuthorizer"]
  F["BearerRequestAuthorizer<br/>fixed typo + logger"] --> E

Loading

File Walkthrough

	Relevant files
Enhancement	TrustValidation.swift New shared trust validation helper                                              Sources/SwiftRestRequests/security/TrustValidation.swift New file created to centralize SecTrust extraction logic Provides extractServerTrust static method that validates and logs missing trust Eliminates code duplication across pinning delegates +37/-0    CertificateCAPinning.swift Refactor to use centralized trust validation                          Sources/SwiftRestRequests/security/CertificateCAPinning.swift Replaced inline serverTrust extraction with TrustValidation.extractServerTrust call Refactored validation logic using guard statement instead of if-else Standardized error and success log messages for consistency Improved code readability and reduced duplication +6/-7      PublicKeyServerPinning.swift Refactor to use centralized trust validation                          Sources/SwiftRestRequests/security/PublicKeyServerPinning.swift Replaced inline serverTrust extraction with TrustValidation.extractServerTrust call Simplified public key validation logic using nested guard statements Standardized error and success log messages for consistency Removed redundant error handling branches +12/-17  URLRequestAuthorizer.swift Add optional logger injection and fix typo                              Sources/SwiftRestRequests/security/URLRequestAuthorizer.swift Added optional logger parameter to BasicRequestAuthorizer constructor with default value Added optional logger parameter to BearerRequestAuthorizer constructor with default value Fixed typo: renamed BearerReqeustAuthorizer to BearerRequestAuthorizer Changed logger from hardcoded instance to injectable dependency Updated documentation to reflect new logger parameter +11/-6
Bug fix	RestApiCallerSecurityTests.swift Update test for renamed authorizer class                                  Tests/SwiftRestRequestsTests/RestApiCallerSecurityTests.swift Updated test to use corrected class name BearerRequestAuthorizer Maintains existing test functionality with fixed authorizer instantiation +1/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit ea8a853)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Audit logging: Security-relevant decisions (trust extraction failure, pinning success/failure) are logged, but it is unclear whether all critical actions across the module are consistently logged with user/action context as required by comprehensive audit trails. Referred Code guard let serverTrust = TrustValidation.extractServerTrust(from: challenge, logger: logger) else { return (.cancelAuthenticationChallenge, nil) } // Set the pinned CA certificates for validation SecTrustSetAnchorCertificates(serverTrust, pinnedCACertificates as CFArray) SecTrustSetAnchorCertificatesOnly(serverTrust, true) // Perform certificate chain validation var error: CFError? = nil let status = SecTrustEvaluateWithError(serverTrust, &error) guard error == nil, status else { logger.error("Security: CA pinning failed - rejecting connection.") return (.cancelAuthenticationChallenge, nil) } logger.info("Security: CA pinning succeeded.") return (.useCredential, URLCredential(trust: serverTrust)) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge cases: Guard-based flow improves clarity, but the pinning and trust extraction paths cancel without surfacing actionable error context to callers, which may limit upstream handling and monitoring. Referred Code guard let serverTrust = TrustValidation.extractServerTrust(from: challenge, logger: logger) else { return (.cancelAuthenticationChallenge, nil) } guard let serverPublicKey = SecTrustCopyKey(serverTrust) else { logger.error("Security: public key pinning failed - no key present in server trust.") return (.cancelAuthenticationChallenge, nil) } guard pinnedPublicKeys.contains(where: { $0 == serverPublicKey }) else { logger.error("Security: public key pinning failed - key not found in pinned set.") return (.cancelAuthenticationChallenge, nil) } logger.info("Security: public key pinning succeeded.") return (.useCredential, URLCredential(trust: serverTrust)) } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit 56d04c5

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Limited Audit Context: Security-relevant pinning decisions are logged but entries lack user/session identifiers and full action context required for comprehensive audit trails. Referred Code logger.info("Initialized CertificateCAPinning", metadata: [ "pinnedCACertificates": "\(pinnedCACertificates)" ]) } /// Validates the server trust against the pinned CA certificates. public func urlSession(_ session: URLSession, didReceive challenge: URLAuthenticationChallenge) async -> (URLSession.AuthChallengeDisposition, URLCredential?) { guard let serverTrust = TrustValidation.extractServerTrust(from: challenge, logger: logger) else { return (.cancelAuthenticationChallenge, nil) } // Set the pinned CA certificates for validation SecTrustSetAnchorCertificates(serverTrust, pinnedCACertificates as CFArray) SecTrustSetAnchorCertificatesOnly(serverTrust, true) // Perform certificate chain validation var error: CFError? = nil let status = SecTrustEvaluateWithError(serverTrust, &error) guard error == nil, status else { ... (clipped 8 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Generic Failure Handling: Pinning failures cancel the challenge with logs but do not propagate actionable error context upstream, which may limit debugging and monitoring. Referred Code public final class PublicKeyServerPinning: NSObject, URLSessionDelegate { let pinnedPublicKeys: [SecKey] let logger = Logger.SwiftRestRequests.security /// Creates a new pinning delegate. /// - Parameter pinnedPublicKeys: Collection of allowed server public keys. public init(pinnedPublicKeys: [SecKey]) { self.pinnedPublicKeys = pinnedPublicKeys } /// Validates the server certificate's public key against the pinned keys. public func urlSession(_ session: URLSession, didReceive challenge: URLAuthenticationChallenge) async -> (URLSession.AuthChallengeDisposition, URLCredential?) { guard let serverTrust = TrustValidation.extractServerTrust(from: challenge, logger: logger) else { return (.cancelAuthenticationChallenge, nil) } guard let serverPublicKey = SecTrustCopyKey(serverTrust) else { logger.error("Security: public key pinning failed - no key present in server trust.") ... (clipped 11 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Authorization Logged: Trace logs include the full Authorization header value which may expose sensitive credentials or tokens in logs. Referred Code /// Applies the precomputed Basic authorization header to the request. public func configureAuthorizationHeader(for urlRequest: inout URLRequest) { logger.trace("Set HTTP Authorization header", metadata: [ "urlRequest": "\(String(describing: urlRequest.url?.absoluteString))", "Authorization": "\(headerValue)"]) urlRequest.setValue(self.headerValue, forHTTPHeaderField: "Authorization") Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Sensitive Header Exposure: Code constructs Authorization headers and logs them verbatim, which risks sensitive data exposure despite otherwise careful security handling. Referred Code /// Applies the precomputed Basic authorization header to the request. public func configureAuthorizationHeader(for urlRequest: inout URLRequest) { logger.trace("Set HTTP Authorization header", metadata: [ "urlRequest": "\(String(describing: urlRequest.url?.absoluteString))", "Authorization": "\(headerValue)"]) urlRequest.setValue(self.headerValue, forHTTPHeaderField: "Authorization") Learn more about managing compliance generic rules or creating your own custom rules

Compliance check up to commit 160f225

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
🔴	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Sensitive data logging: The code logs the Authorization header including credentials for Basic auth, which constitutes sensitive data exposure in logs. Referred Code logger.trace("Set HTTP Authorization header", metadata: [ "urlRequest": "\(String(describing: urlRequest.url?.absoluteString))", "Authorization": "\(headerValue)"]) urlRequest.setValue(self.headerValue, forHTTPHeaderField: "Authorization")
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action logging scope: New code adds trace/info/error logs for security-related operations, but it is unclear whether all critical actions across the system are logged with user identity and full context per audit requirements. Referred Code logger.trace("Set HTTP Authorization header", metadata: [ "urlRequest": "\(String(describing: urlRequest.url?.absoluteString))", "Authorization": "\(headerValue)"]) urlRequest.setValue(self.headerValue, forHTTPHeaderField: "Authorization")
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs may reveal: The Basic authorizer logs the full Authorization header value which appears to include the base64 credentials, potentially exposing sensitive data in logs. Referred Code logger.trace("Set HTTP Authorization header", metadata: [ "urlRequest": "\(String(describing: urlRequest.url?.absoluteString))", "Authorization": "\(headerValue)"]) urlRequest.setValue(self.headerValue, forHTTPHeaderField: "Authorization")

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Validate certificate chain before pinning Add a call to SecTrustEvaluateWithError to validate the certificate chain before extracting and pinning the public key in PublicKeyServerPinning.swift. Sources/SwiftRestRequests/security/PublicKeyServerPinning.swift [51-54] +var error: CFError? +guard SecTrustEvaluateWithError(serverTrust, &error), error == nil else { + logger.error("Security: Trust evaluation failed before public key pinning. Error: \(String(describing: error))") + return (.cancelAuthenticationChallenge, nil) +} + guard let serverPublicKey = SecTrustCopyKey(serverTrust) else { logger.error("Security: public key pinning failed - no key present in server trust.") return (.cancelAuthenticationChallenge, nil) } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical security vulnerability where public key pinning is performed without first validating the certificate chain, which could allow a man-in-the-middle attack.	High
Update