Skip to content

Fix/status gate - #517

Open
eric-pSAP wants to merge 4 commits into
mainfrom
fix/status-gate
Open

Fix/status gate#517
eric-pSAP wants to merge 4 commits into
mainfrom
fix/status-gate

Conversation

@eric-pSAP

@eric-pSAP eric-pSAP commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Duplicate of #510 to allow for merging.

@eric-pSAP
eric-pSAP requested a review from a team as a code owner August 6, 2026 08:45
@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

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


Fix: Malware Scan-Status Gate Bypassed by /$value URL Suffix

Bug Fix

🐛 Resolved a security gap where the malware scan-status gate could be bypassed when accessing attachment content via OData's /$value suffix (e.g., /content/$value). The previous regex and endsWith checks did not account for this suffix, allowing infected attachments to be downloaded without triggering the scan policy enforcement (CWE-184).

Changes

  • lib/helper.js: Introduced two new utility functions:

    • isContentRequest(url): Uses a regex that correctly matches /content, /_content, and their /$value variants, stripping query strings and fragments before matching.
    • extractContentPrefix(url): Properly extracts the field prefix from _content URLs, including those ending in /$value.
      Both functions are exported via module.exports.
  • lib/generic-handlers.js: Replaced the inline endsWith('/content') and /_content$/ checks with calls to isContentRequest(), and replaced manual .split("/").pop().replace("_content", "") prefix extraction with extractContentPrefix(). This fixes the gate bypass for /$value URLs in both validateAttachment and readAttachment.

  • CHANGELOG.md: Added an [Unreleased] entry documenting the fix, referencing CWE-184.

  • tests/unit/status-gate.test.js: Added integration-style unit tests verifying that validateAttachment correctly rejects infected attachments for /content, /content/$value, and /foo_content/$value URLs.

  • tests/unit/contentUrlHelpers.test.js: Added comprehensive unit tests for the new isContentRequest and extractContentPrefix helper functions, covering standard paths, /$value variants, query strings, fragments, and edge cases (null/undefined).


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

Version: 1.29.18

@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 introduces a well-structured fix with good test coverage, but there is one real bug: both isContentRequest and extractContentPrefix will throw a TypeError when passed null or undefined because the optional chaining in url?.split("?")[0] only guards the .split("?") call — the subsequent [0] bracket access on the resulting undefined still throws. The tests that assert safe handling of null/undefined inputs will therefore fail rather than pass. A second ?. before .split("#") fixes both functions, as noted in the inline comment.

PR Bot Information

Version: 1.29.18

  • Event Trigger: pull_request.opened
  • LLM: anthropic--claude-4.6-sonnet
  • Correlation ID: 2ee0d870-9173-11f1-89c9-b41bb9ba7b23
  • File Content Strategy: Full file content

Comment thread lib/helper.js Outdated
Co-authored-by: hyperspace-pr-bot[bot] <209611008+hyperspace-pr-bot[bot]@users.noreply.github.com>
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