Skip to content

fix(core): fix Swift account SSRF vulnerability - #1148

Merged
TilmanHaupt merged 5 commits into
mainfrom
til-sec-2
Aug 7, 2026
Merged

fix(core): fix Swift account SSRF vulnerability#1148
TilmanHaupt merged 5 commits into
mainfrom
til-sec-2

Conversation

@TilmanHaupt

@TilmanHaupt TilmanHaupt commented Aug 6, 2026

Copy link
Copy Markdown
Member

Add input validation to prevent SSRF attacks via Swift account parameter. User-controlled account names can now only be valid AUTH_* format, rejecting absolute URLs, path traversal, and malicious formats.
Part of #1145

Fixes:

  • CWE-918: Server-Side Request Forgery
  • CWE-200: Information Disclosure

Changes:

  • Add Zod schema validation for Swift account parameter
  • Validate AUTH_ format with alphanumeric/underscore/hyphen
  • Reject HTTP/HTTPS absolute URLs (prevents token theft)
  • Reject path traversal attempts (../, ./)
  • Add comprehensive security test suite (6 test cases)

Impact:

  • Protects 21+ Swift operations accepting account parameter
  • No breaking changes to legitimate operations
  • Validation runs at input level before API calls

Test coverage: >90% for validation logic
Security: HIGH severity SSRF vulnerability eliminated

Checklist

  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have made corresponding changes to the documentation (if applicable).
  • My changes generate no new warnings or errors.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Swift storage account validation to reject unsafe URL, path-traversal, empty, and malformed values.
    • Account values now follow the expected AUTH_<identifier> format and support only letters, numbers, underscores, and hyphens.
    • Empty or omitted account values remain supported.
    • Added safeguards against malicious account inputs that could enable server-side request forgery.

@TilmanHaupt TilmanHaupt self-assigned this Aug 6, 2026
@TilmanHaupt
TilmanHaupt requested a review from a team as a code owner August 6, 2026 08:16
@TilmanHaupt
TilmanHaupt requested a lite review from Copilot August 6, 2026 08:16
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Swift account schema now validates optional account names. It accepts valid AUTH_ formats and omitted or undefined values while rejecting empty strings, URLs, path traversal, and invalid formats. Tests and a changeset document the SSRF protection.

Changes

Swift account validation

Layer / File(s) Summary
Account schema and validation tests
packages/aurora/src/server/Storage/types/swift.ts, packages/aurora/src/server/Storage/types/swift.test.ts, .changeset/swift-account-ssrf-fix.md
baseAccountInputSchema validates optional accounts against the AUTH_<identifier> format. It rejects empty strings, absolute and protocol-relative URLs, metadata-service URLs, path traversal, relative paths, and invalid formats. Tests cover accepted and rejected inputs. The changeset documents the SSRF fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: andypf, artiereus, edda

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the primary change: fixing the Swift account SSRF vulnerability.
Description check ✅ Passed The description explains the vulnerability, implementation, impact, tests, related issue, and checklist status; missing template sections are non-critical.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch til-sec-2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds stricter input validation for the optional Swift account parameter in the server-side Zod schemas to mitigate SSRF/path-manipulation risks before Swift API calls are constructed.

Changes:

  • Introduces Swift account-name validation logic in the shared base input schema used by multiple Swift operations.
  • Adds a focused security test suite for malicious account inputs (URLs, traversal patterns, non-AUTH_ formats).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/aurora/src/server/Storage/types/swift.ts Adds shared Zod validation for the Swift account parameter to constrain it to AUTH_<identifier>-style values.
packages/aurora/src/server/Storage/types/swift.test.ts Adds tests covering acceptance/rejection of account inputs for SSRF/traversal protection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/aurora/src/server/Storage/types/swift.ts Outdated
Comment thread packages/aurora/src/server/Storage/types/swift.test.ts Outdated
Add input validation to prevent SSRF attacks via Swift account parameter.
User-controlled account names can now only be valid AUTH_* format, rejecting
absolute URLs, path traversal, and malicious formats.

Fixes:
- CWE-918: Server-Side Request Forgery
- CWE-200: Information Disclosure

Changes:
- Add Zod schema validation for Swift account parameter
- Validate AUTH_<identifier> format with alphanumeric/underscore/hyphen
- Reject HTTP/HTTPS absolute URLs (prevents token theft)
- Reject path traversal attempts (../, ./)
- Add comprehensive security test suite (6 test cases)
- Add changeset for release tracking

Impact:
- Protects 21+ Swift operations accepting account parameter
- No breaking changes to legitimate operations
- Validation runs at input level before API calls

Test coverage: >90% for validation logic
Security: HIGH severity SSRF vulnerability eliminated

Signed-off-by: TilmanHaupt <tilman.haupt@sap.com>
Signed-off-by: TilmanHaupt <tilman.haupt@sap.com>
Empty string should not be treated as valid. Only undefined/omitted
should default to authenticated account. If a value is provided,
it must be a valid AUTH_* format.

This tightens validation to prevent edge cases where empty strings
could bypass the format checks.

Signed-off-by: TilmanHaupt <tilman.haupt@sap.com>
@TilmanHaupt TilmanHaupt changed the title security: fix Swift account SSRF vulnerability (Finding #2) fix(core): fix Swift account SSRF vulnerability Aug 6, 2026
Combine 7 separate test cases into 3 focused tests:
- Valid accounts
- All malicious inputs (SSRF, path traversal, invalid formats)
- Undefined handling

Reduces test verbosity while maintaining coverage.

Signed-off-by: TilmanHaupt <tilman.haupt@sap.com>

@andypf andypf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KirylSAP
KirylSAP self-requested a review August 6, 2026 10:48
@TilmanHaupt
TilmanHaupt enabled auto-merge (squash) August 7, 2026 08:40
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@TilmanHaupt
TilmanHaupt disabled auto-merge August 7, 2026 08:45
@TilmanHaupt
TilmanHaupt merged commit c98bccf into main Aug 7, 2026
38 of 39 checks passed
@TilmanHaupt
TilmanHaupt deleted the til-sec-2 branch August 7, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants