Remove Inline Type from index.cds

[eric-pSAP]

Security fix where inline type was overwriting CAP safety and allowed executables to run on download. Line was remove to allow default security behavior since no previewing was done previously.

[hyperspace-pr-bot]

Looking at the key finding from lib/plugin.js line 305, @Core.ContentDisposition.Type is still explicitly set to 'inline' for single inline attachments in unfoldModel. The CDS annotation removal in db/index.cds only removes it from composition-based attachments (Attachments aspect), so inline attachments remain unaffected by this fix. This is correct scoped behavior.

The main substantive issue found is the missing try/finally guard already posted. The overall PR is a focused, well-motivated security fix: the Type: 'inline' annotation on composition-based attachments was overriding CAP's default content-disposition: attachment behavior, enabling stored XSS via SVG uploads. The accompanying test correctly validates the fix. The only gap is the cleanup pattern for the scan flag, which has been flagged above.

PR Bot Information

Version: 1.28.0

• LLM: anthropic--claude-4.6-sonnet
• Event Trigger: pull_request.opened
• Correlation ID: c25552a0-8414-11f1-955b-637722c8df04
• File Content Strategy: Full file content

[hyperspace-pr-bot]

Summary

The following content is AI-generated and provides a summary of the pull request:

---

Security Fix: Remove Inline Content Disposition to Prevent XSS via SVG Attachments

Bug Fix

🐛 Removes the hardcoded Type: 'inline' from the @Core.ContentDisposition annotation in db/index.cds, which was overriding CAP's default security behavior. By forcing inline content disposition, browsers would render SVG attachments directly — allowing embedded scripts to execute, creating a stored XSS vulnerability. Removing this line restores the default attachment disposition, forcing browsers to download files rather than render them.

Changes

• db/index.cds: Removed Type: 'inline' from the @Core.ContentDisposition annotation on the content field, leaving only Filename: filename. This ensures file downloads use content-disposition: attachment by default.
• tests/integration/attachments.test.js: Added a new integration test "Stored XSS: SVG attachment must be served with content-disposition: attachment, not inline" that uploads a malicious SVG payload and asserts the response header uses attachment and not inline.
• CHANGELOG.md: Added entry for version 3.13.2 documenting this fix.

---

• 🔄 Regenerate and Update Summary
• ✏️ Insert as PR Description (deletes this comment)
• 🗑️ Delete comment

PR Bot Information

Version: 1.28.0

• Correlation ID: 07c60180-8416-11f1-8b11-2c4e6795db7e
• LLM: anthropic--claude-4.6-sonnet
• File Content Strategy: Full file content
• Event Trigger: pull_request.ready_for_review
• Summary Prompt: Default Prompt
• Output Template: Default Template

[hyperspace-pr-bot]

Now I have full context. Let me look at the test around line 2009 more carefully. I need to check if cds.env.requires.attachments.scan = false is placed before the try block at line 2009.

Looking at the diff:

• Line 2009: cds.env.requires.attachments.scan = false — this is outside the try block (the try starts at line 2011)
• Line 2063: } finally {
• Line 2064: cds.env.requires.attachments.scan = true

The previous reviewer already flagged the issue where scan is not restored if the test fails before line 2063 — specifically noting that scan = false is set at line 2009, before the try block at line 2011. The existing comment covers this exact problem.

Now let me look for other issues in the diff I haven't commented on yet:

1. db/index.cds: The Type: 'inline' line was removed from @Core.ContentDisposition. This is the core security fix. The @Core.ContentDisposition annotation now only specifies Filename — no Type. This means the default CAP behavior (which should force attachment) will be used. This looks correct.

2. Test: At line 2062, expect(disposition.toLowerCase()).not.toMatch(/^inline/) — this check is redundant given line 2061 already asserts the header starts with attachment. But it's not a defect.

3. The try/finally structure: The scan is set to false at line 2009, before the try block. The finally block at 2063 restores it. The existing comment already captures this issue.

Let me check if there are any other issues I might have missed — specifically the CHANGELOG.md change and the assertion logic.

Looking at test line 2061: expect(disposition.toLowerCase()).toMatch(/^attachment/) — If the disposition header is empty string "" (e.g., not set at all), the ?? "" fallback means disposition is "", which would fail the .toMatch(/^attachment/) check. This is correct behavior — if the header is missing, the test would correctly fail. So that's fine.

The existing comment I already posted covers the main bug (scan flag restoration), which is already noted. The suggestion in the existing comment already proposes the fix (move scan = false inside the try block). I should not re-raise that.

There are no new substantive issues beyond what's already been flagged. The core change (removing Type: 'inline' from @Core.ContentDisposition) is the security fix described in the PR, and it appears correct.

The PR makes a focused, correct security fix by removing the Type: 'inline' annotation from @Core.ContentDisposition in db/index.cds, which previously overrode CAP's default safe behavior. The accompanying test is well-structured, though the existing comment already identified the one defect (the scan flag assignment at line 2009 being outside the try block). Please address that comment before merging.

PR Bot Information

Version: 1.28.0

• Correlation ID: 07c60180-8416-11f1-8b11-2c4e6795db7e
• LLM: anthropic--claude-4.6-sonnet
• File Content Strategy: Full file content
• Event Trigger: pull_request.ready_for_review

[patricebender]

would it be possible to ship a release after merging this? Thanks!