Skip to content

feat(POS-1338): Implement ActionHandler instance binding (remedy cross-contamination) #190

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.

Sign up for GitHub

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

Jump to bottom
Draft
mateusz-walesiak-cko wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat(POS-1338): Implement ActionHandler instance binding (remedy cross-contamination) #190

mateusz-walesiak-cko wants to merge 1 commit into from

Conversation

@mateusz-walesiak-cko
Copy link
Contributor

@mateusz-walesiak-cko mateusz-walesiak-cko commented Jun 2, 2025 •
edited
Loading

Description

Implement safeguards similar to these in CardField into ActionHandler to prevent race conditions and cross-contamination between multiple tabs/instances. Here's what has been added:

🔧 Key Changes Made:

  • Instance-Specific Unique Identifiers (instanceUID)
  • data.frameID validation against this.instanceUID
  • New Public Methods for Resource Management
    • setResourceID(); informational for now
    • getInstanceUID() ;

🛡️ Security Improvements:

  • Instance Isolation: Each ActionHandler now has a unique identifier that prevents messages from other instances being processed
  • Resource ID Tracking: Added capability to track which resource (invoice) an ActionHandler is associated with
  • Frame ID Validation: Messages from the processout.checkout namespace are only processed if they have the correct frameID matching the instanceUID

📋 Next steps:
To fully implement this solution, we have to update the checkout pages' postMessage responses with the frameID

Solution

Demo

Checklist

  • I bumped the version of the project using yarn bump-version
  • I have checked the code for any potential issues
  • I tested my changes in the browser

Jira Issue

https://checkout.atlassian.net/browse/POS-1338

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 2, 2025
View reviewed changes
src/processout/actionhandler.ts
if (!this.options) this.options = new ActionHandlerOptions();

// Generate unique identifier for this instance to prevent cross-contamination
this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
Loading
in a security context.

Copilot Autofix

AI 6 months ago

To fix the issue, replace the use of Math.random() with a cryptographically secure random number generator. In Node.js, the crypto module provides the randomBytes method, which can be used to generate secure random values. The fix involves:

  1. Importing the crypto module.
  2. Using crypto.randomBytes to generate a secure random value.
  3. Converting the random bytes into a string format similar to the original implementation.

The replacement ensures that the instanceUID is generated securely while maintaining the original functionality.

Suggested changeset 1
src/processout/actionhandler.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/src/processout/actionhandler.ts b/src/processout/actionhandler.ts
--- a/src/processout/actionhandler.ts
+++ b/src/processout/actionhandler.ts
@@ -1,2 +1,3 @@
 /// <reference path="../references.ts" />
+import * as crypto from 'crypto';
 
@@ -226,3 +227,4 @@
             // Generate unique identifier for this instance to prevent cross-contamination
-            this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;
+            const randomBytes = crypto.randomBytes(16); // Generate 16 random bytes
+            this.instanceUID = `action_${randomBytes.toString('hex').substring(0,7)}`;
             
EOF
@@ -1,2 +1,3 @@
/// <reference path="../references.ts" />
import * as crypto from 'crypto';

@@ -226,3 +227,4 @@
// Generate unique identifier for this instance to prevent cross-contamination
this.instanceUID = `action_${Math.random().toString(36).substring(7)}`;
const randomBytes = crypto.randomBytes(16); // Generate 16 random bytes
this.instanceUID = `action_${randomBytes.toString('hex').substring(0,7)}`;

Copilot is powered by AI and may make mistakes. Always verify output.
@mateusz-walesiak-cko mateusz-walesiak-cko force-pushed the feat/POS-1338/actionhandler-instance-binding branch from 6f16fef to 0840163 Compare June 2, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mateusz-walesiak-cko