Skip to content
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

bugfix/CXSPA- 6934 & 9292: CDC Consent Management Issue #19916

Draft
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

anjana-bl
Copy link
Contributor

@anjana-bl anjana-bl commented Jan 23, 2025

JIRA:

  1. https://jira.tools.sap/browse/CXSPA-6934 - Reconsent popup doesnot distinguish between mandatory and non-mandatory consents
  2. https://jira.tools.sap/browse/CXSPA-9291 - During registration distinguish between mandatory and non-mandatory consents and save only the provided preferences

@anjana-bl anjana-bl requested review from a team as code owners January 23, 2025 03:16
@github-actions github-actions bot marked this pull request as draft January 23, 2025 03:17
@anjana-bl anjana-bl changed the title bugfix/CXSPA- XXXX & XXXX: CDC Consent Management Issue bugfix/CXSPA- 6934 & XXXX: CDC Consent Management Issue Jan 23, 2025
@anjana-bl anjana-bl changed the title bugfix/CXSPA- 6934 & XXXX: CDC Consent Management Issue bugfix/CXSPA- 6934 & 9292: CDC Consent Management Issue Jan 23, 2025
) {
target[key] = this.deepMerge(target[key] ?? {}, source[key]);
} else {
target[key] = source[key];

Check warning

Code scanning / CodeQL

Prototype-polluting function Medium

Properties are copied from
source
to
target
without guarding against prototype pollution.

Copilot Autofix AI 3 days ago

To fix the prototype pollution vulnerability, we need to ensure that the deepMerge function does not copy the special properties __proto__ and constructor. This can be achieved by adding checks to skip these properties during the merge process.

  • Modify the deepMerge function to include checks for __proto__ and constructor.
  • Ensure that these properties are not copied from the source object to the target object.
Suggested changeset 1
integration-libs/cdc/root/consent-management/converters/cdc-preference.serializer.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/integration-libs/cdc/root/consent-management/converters/cdc-preference.serializer.ts b/integration-libs/cdc/root/consent-management/converters/cdc-preference.serializer.ts
--- a/integration-libs/cdc/root/consent-management/converters/cdc-preference.serializer.ts
+++ b/integration-libs/cdc/root/consent-management/converters/cdc-preference.serializer.ts
@@ -44,2 +44,5 @@
     for (const key of Object.keys(source)) {
+      if (key === '__proto__' || key === 'constructor') {
+        continue;
+      }
       if (
EOF
@@ -44,2 +44,5 @@
for (const key of Object.keys(source)) {
if (key === '__proto__' || key === 'constructor') {
continue;
}
if (
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@@ -7,6 +7,10 @@
import { Injectable } from '@angular/core';
import { ConsentTemplate, Converter } from '@spartacus/core';

/**
* @deprecated since 2211-ng19.0, use class CdcPreferenceSerializer instead
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI It's still debatable whether the Feb release will be named 2211-ng19.0. We're checking now if it's possible, due to being incompatible with the SemVer schema https://semver.org/ - which is assumed in the npm ecosystem

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