fix(files_external): sanitize exception messages in storage status response

[DeepDiver1975]

Summary

• The \Exception catch block in updateStorageStatus() returned raw exception messages (including Guzzle cURL error 7: Failed to connect to 10.0.0.x) in the JSON statusMessage field
• Authenticated users with storage access could use connection error differences to map internal network topology (SSRF oracle)
• Fix: log full exception server-side; return generic l10n string to client

Security Impact

Medium — internal network reconnaissance oracle for authenticated users with external storage access

Note

This PR touches StoragesController.php — merge security/fix-files-external-ssrf first to avoid conflicts.

Test plan

• testUpdateStorageStatusDoesNotLeakExceptionDetailsInStatusMessage asserts IP addresses and exception class names do not appear in statusMessage; fails without fix
• Run make test TEST_PHP_SUITE=apps/files_external

🤖 Generated with Claude Code

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[DeepDiver1975]

Code Review — fix(files_external): sanitize exception messages in storage status response

Overview: Replaces the raw get_class($e).': '.$e->getMessage() string in the external storage status error response with a generic localized message, preventing internal network details (IPs, ports, cURL errors) from leaking to authenticated clients.

Correctness

The fix is correct. The full exception is now logged server-side via $this->logger->logException() before the status is set, so no diagnostic information is lost — it's just no longer sent to the client.

The localized message 'Storage connection error. See server log for details.' is appropriate. The $this->l10n->t() call ensures it passes through the translation layer.

Test

The new test testUpdateStorageStatusDoesNotLeakExceptionDetailsInStatusMessage is thorough:

• Uses a realistic $sensitiveMessage mimicking a Guzzle cURL error with an internal IP
• Asserts that none of: the raw message, the class name, the internal IP, or "RuntimeException" appear in statusMessage
• Correctly checks STATUS_ERROR is still set (error is surfaced, just not detailed)

One note: the test uses \OC_Mount_Config::$skipTest = false to force the real exception path. Resetting to true in the finally-equivalent teardown is important — the test does restore it after, which is correct.

Summary

Aspect	Assessment
Security fix	✅ Information disclosure eliminated
Logging	✅ Full exception still logged server-side
Tests	✅ Directly asserts no sensitive strings in response

Verdict: Ready to merge.

[DeepDiver1975]

Code Review

Overview: Security fix — stops raw exception messages (including internal IP addresses, ports, and cURL error details) from being returned to authenticated clients via the external storage status API.

The vulnerability: In StoragesController::updateStorageStatus(), the catch block set the storage status message to get_class($e).': '.$e->getMessage(). When ownCloud attempts to connect to a remote storage and Guzzle throws a ConnectException, the message includes the resolved IP and port (e.g. cURL error 7: Failed to connect to 10.0.0.99 port 80). This is returned in the JSON response, letting any authenticated user with access to the storage settings page map internal network topology — a classic SSRF oracle.

The fix: The exception is now logged server-side via $this->logger->logException() and the client receives a generic translated message ('Storage connection error. See server log for details.') instead.

Test: testUpdateStorageStatusDoesNotLeakExceptionDetailsInStatusMessage is a thorough regression test. It:

• Injects a RuntimeException with a realistic sensitive payload (cURL error 7: Failed to connect to 10.0.0.99 port 80)
• Asserts the response status is STATUS_ERROR
• Asserts the statusMessage does not contain the raw exception message, the exception class name, or the internal IP address

One note on test implementation: the test calls \OC_Mount_Config::$skipTest = false to exercise the real exception path, then resets it. This relies on a test-infrastructure flag that's somewhat fragile, but it's an existing pattern in this test class.

Changelog: changelog/unreleased/41585 correctly categorises this as Security and accurately describes the information-disclosure risk.

No blocking concerns. Clean fix with good test coverage.