Skip to content

Remove inline annotation and add document the new behavior#872

Merged
Schmarvinius merged 5 commits into
mainfrom
fix/security
Jul 23, 2026
Merged

Remove inline annotation and add document the new behavior#872
Schmarvinius merged 5 commits into
mainfrom
fix/security

Conversation

@Schmarvinius

@Schmarvinius Schmarvinius commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Security: Default Content-Disposition to attachment to Mitigate Stored XSS

Security Fix

🔒 Changed the default Content-Disposition for attachment content from inline to attachment. This mitigates stored XSS attacks (CWE-79) where user-uploaded SVG or HTML payloads could be executed in the application origin when rendered inline by the browser.

Applications that require inline previews can now explicitly opt in via their own CDS model annotation. It is strongly recommended to combine inline disposition with @Core.AcceptableMediaTypes to restrict content to non-scriptable types (e.g., image/jpeg, image/png, application/pdf).

Changes

  • attachments-annotations.cds: Removed Type: 'inline' from the Core.ContentDisposition annotation, so attachments now default to attachment disposition. The Filename property is retained.
  • README.md: Added a new Content Disposition section to the Table of Contents and documentation, explaining the new default behavior, the XSS security rationale, and how to opt in to inline rendering with a security warning.
  • doc/CHANGELOG.md: Added an Unreleased section documenting the security change, referencing cds-calesi #1263, and providing guidance for applications that need inline previews.
  • OdataRequestValidationWithoutTestHandlerAndWithoutMalwareScannerTest.java: Added a regression test (contentDownloadUsesAttachmentDisposition) that verifies attachment content responses default to Content-Disposition: attachment, guarding against regressions of the XSS mitigation. Also updated the copyright year.
  • 🔄 Regenerate and Update Summary
PR Bot Information

Version: 1.29.2

  • Event Trigger: pull_request.opened
  • Output Template: Default Template
  • Summary Prompt: Default Prompt
  • Correlation ID: 20d41ca0-867a-11f1-80b4-078f025b1838
  • File Content Strategy: Full file content
  • LLM: anthropic--claude-4.6-sonnet

@Schmarvinius
Schmarvinius requested a review from a team as a code owner July 23, 2026 09:37

@hyperspace-pr-bot hyperspace-pr-bot Bot 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.

The PR makes a focused and well-motivated security fix — removing the Content-Disposition: inline default to mitigate stored XSS — with matching documentation, a changelog entry, and a regression test. The main issues to address are: the integration test performs a redundant second HTTP request instead of reusing the already-verified Awaitility response, the doesNotStartWithIgnoringCase("inline") assertion is logically redundant given the preceding startsWith("attachment") check, and the README security warning should qualify application/pdf as a potentially scriptable type in certain viewers.

PR Bot Information

Version: 1.29.2

  • Event Trigger: pull_request.opened
  • Experiment Variant: control
  • Correlation ID: 20d41ca0-867a-11f1-80b4-078f025b1838
  • File Content Strategy: Full file content
  • Agent Instructions:
  • LLM: anthropic--claude-4.6-sonnet

Comment thread README.md Outdated
@Schmarvinius
Schmarvinius merged commit 73d3358 into main Jul 23, 2026
20 checks passed
@Schmarvinius
Schmarvinius deleted the fix/security branch July 23, 2026 12:23
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.

2 participants