Skip to content

Fix: wrong error message when file not yet uploaded - #521

Open
eric-pSAP wants to merge 5 commits into
mainfrom
scanMessageFix
Open

Fix: wrong error message when file not yet uploaded#521
eric-pSAP wants to merge 5 commits into
mainfrom
scanMessageFix

Conversation

@eric-pSAP

@eric-pSAP eric-pSAP commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fix: Return 404 When Attachment File Has Not Yet Been Uploaded

Bug Fix

🐛 Previously, when a client attempted to download an attachment whose metadata record existed (e.g., created via a POST) but for which no actual file had been uploaded yet, the wrong error was returned. The attachment was treated as "Unscanned" and triggered a rescan flow (resulting in a 202 response) instead of returning a proper 404 Not Found error.

This fix adds a content check for Unscanned attachments: if neither a url (object-store) nor inline content (DB-based) is present, the handler now returns 404 immediately without triggering a malware scan.

Changes

  • lib/generic-handlers.js: In getScanInfo, the url field is now extracted from AttachmentsSrv.getStatus. For Unscanned attachments, a content existence check is performed — if no file content is found (url is null and no content column entry exists), the status is returned as null, causing downstream logic to reject with 404.

  • srv/attachments/basic.js: Updated getStatus to include the url field in both the SELECT query and the returned object. Updated JSDoc to reflect the new return type (url: string|null).

  • tests/unit/rejectionEvents.test.js: Updated existing Unscanned test mocks to include a non-null url, preserving their original behavior. Added a new describe block ("Download rejected when no file has been uploaded") with two test cases validating that a 404 is returned when url is null — both when scanning is enabled and when it is disabled.

  • package.json: Pinned @cap-js/cds-test dev dependency to ^1.0.2 (previously ^1).

  • 🔄 Regenerate and Update Summary
PR Bot Information

Version: 1.31.20

  • Correlation ID: 143322e0-abb2-11f1-9ba1-7cf116448758
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • Summary Prompt: Default Prompt
  • Output Template: Default Template
  • LLM: anthropic--claude-4.6-sonnet

@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.

I found a blocking correctness issue in the new content-existence check: it trusts url as proof of uploaded content, but URLs are created during metadata preparation as well, so metadata-only object-store attachments can still be treated as downloadable/rescannable instead of returning 404. Since no further tool calls are allowed, I could not post the inline comment; please update the check to verify actual object existence for object-store backends, or fall back to checking DB content for DB-backed storage.

PR Bot Information

Version: 1.31.20

  • Correlation ID: 143322e0-abb2-11f1-9ba1-7cf116448758
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • Experiment Variant: treatment
  • LLM: gpt-5.5

@eric-pSAP
eric-pSAP marked this pull request as ready for review September 9, 2026 18:02
@eric-pSAP
eric-pSAP requested a review from a team as a code owner September 9, 2026 18:02
@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

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


Return 404 for Metadata-Only Attachment Downloads

Bug Fix

🐛 Fixed attachment download/rescan handling for records that exist as metadata only, but do not yet have uploaded file content. Unscanned attachments without object-store url or inline DB content now correctly return 404 Not Found instead of triggering malware rescan logic and returning a misleading 202.

Changes

  • lib/generic-handlers.js: Enhanced getScanInfo to retrieve attachment url and verify actual file content for Unscanned attachments. Returns a missing-status result when no content exists, allowing downstream logic to reject with 404.
  • srv/attachments/basic.js: Updated getStatus to select and return the attachment url, and adjusted JSDoc for the expanded return shape.
  • tests/unit/rejectionEvents.test.js: Updated existing Unscanned scan/rescan mocks to include uploaded-file URLs, and added coverage ensuring metadata-only attachments return 404 with scanning both enabled and disabled.
  • CHANGELOG.md: Documented the corrected behavior for metadata-only attachment downloads/rescans.
  • package.json: Pinned @cap-js/cds-test to ^1.0.2.
  • tests/incidents-app/package.json: Relaxed the @cap-js/postgres dependency constraint to >=2.2.0.

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

Version: 1.31.27

  • Event Trigger: pull_request.ready_for_review
  • File Content Strategy: Full file content
  • Output Template: Default Template
  • LLM: gpt-5.5
  • Correlation ID: ad01def0-ac78-11f1-961c-c1826cfccab0
  • Summary Prompt: Default Prompt

@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.

I found one substantive issue but couldn’t post the inline comment because the selected snippet did not match the review tool’s diff matching requirements before the tool budget was exhausted.

The main concern is in lib/generic-handlers.js: the new hasContent check treats any non-null url as proof that file content exists. However, finalizePrepareAttachment() appears to create/populate url during metadata preparation, before binary content is necessarily uploaded. That means metadata-only object-store attachments can still be considered as having content and may continue down the rescan/202 path instead of returning 404, which undermines the PR’s stated fix.

PR Bot Information

Version: 1.31.27

  • Correlation ID: ad01def0-ac78-11f1-961c-c1826cfccab0
  • File Content Strategy: Full file content
  • Event Trigger: pull_request.ready_for_review
  • LLM: gpt-5.5

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.

1 participant