Skip to content

Address FN involving CAP remote flow sources #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jeongsoolee09
Copy link
Contributor

What This PR Contributes

This PR fixes false negative cases where the current CAP remote flow source definition UserProvidedPropertyReadOfHandlerParameterOfExposedService is missing detections from the code that comes in the following form:

import cds from "@sap/cds";

module.exports = (srv) => { srv.on(...) }

Two things are notable in the above case:

  1. The code is mixing ES2015 modules and CommonJS modules with regards to importing and exporting values.
  2. The service is implemented in a form of a function (or closure) exported by this module.

Unfortunately, the CodeQL Standard Library for JavaScript cannot detect the above (srv) => { srv.on(...) } element due to some limitations (see issue #221). Therefore, we directly describe the above pattern module.exports = (...) => { ... } as a hack around it.

Other than that, the code changes include:

  • Extension to definitions:
    • Add an explicit case named ExportedClosureApplicationServiceDefinition of a UserDefinedApplicationService that is defined in the form of an exported function whose first parameter represents the service instance.
    • Add a definition ServiceInstanceFromExportedClosureParameter that matches above class.
  • Bug fixes:
    • Address a bug in the UserDefinedApplicationService.isExposed/0 where a simple negation also captured cases where CDS declarations were missing.
    • Address a bug in the HandlerParameterOfExposedService where the trailing .getCdsDeclaration() was missing.

Future Works

Remove the "hack around it" once the relevant portion of the library is patched and released.

…ervice`

- Extension to definitions:
  - Add an explicit case named `ExportedClosureApplicationServiceDefinition` of a `UserDefinedApplicationService` that is defined in the form of an exported function whose first parameter represents the service instance.
  - Add a definition `ServiceInstanceFromExportedClosureParameter` that matches above class.
- Bug fixes:
  - Address a bug in the `UserDefinedApplicationService.isExposed/0` where a simple negation also captured cases where CDS declarations were missing.
  - Address a bug in the `HandlerParameterOfExposedService` where the trailing `.getCdsDeclaration()` was missing.

NOTE: See issue #221 to learn more about the `HACK` block comment.
@jeongsoolee09 jeongsoolee09 self-assigned this Aug 15, 2025
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