Conversation
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
cdd2f3b to
bc77c0c
Compare
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
There was a problem hiding this comment.
Pull request overview
This PR appears to harden cross-frame communication by scoping Framebus traffic to explicit target origins and adding postMessage origin checks, with corresponding test updates.
Changes:
- Add
.target(...)to Framebusemitcalls (primarily targetingproperties.IFRAME_SECURE_ORIGINor client domain). - Add
MessageEvent.originvalidation in internal iframe message handlers. - Update Jest tests/mocks to include
emitonbus.target()mocks and setoriginin syntheticMessageEvents; bump package version.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skyflow.test.ts | Updates Framebus target mocks to include emit and adjusts delete test flow. |
| tests/skyflow.test.js | Updates Framebus target mocks to include emit and minor formatting cleanup. |
| tests/core/internal/skyflow-frame/skyflow-frame-controller.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/internal/skyflow-frame/skyflow-frame-controller.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/internal/skyflow-frame/skyflow-frame-controller-upload-tokenize.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/internal/skyflow-frame/skyflow-frame-controller-upload-tokenize.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/internal/reveal/reveal-frame.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/internal/internal-index.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/internal/iframe-form/iframe-form.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/internal/frame-element-init.test.js | Adds origin to a synthetic MessageEvent for multi-file upload flow. |
| tests/core/internal/frame-element-init.additional.test.js | Adds origin on synthetic events passed into private handler calls. |
| tests/core/internal/composable-frame-element-init.test.js | Adds origin to synthetic MessageEvents used to drive composable height/reveal flows. |
| tests/core/external/reveal/reveal-element.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-element.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-container.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-container.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-composable-internal.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-composable-element.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/external/reveal/reveal-composable-container.test.js | Adds properties import and sets origin in MessageEvents to match production origin checks. |
| tests/core/external/collect/composable-container.test.ts | Adds properties import, adds emit to bus mock, and sets origin in events. |
| tests/core/external/collect/composable-container.test.js | Adds properties import, adds emit to bus mock, sets origin in events, and guards callback extraction. |
| tests/core/external/collect/collect-element.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/external/collect/collect-element.test.js | Adds emit to bus.target() mock return objects. |
| tests/core/external/collect/collect-container.test.ts | Adds emit to bus.target() mock return objects. |
| tests/core/external/collect/collect-container.test.js | Adds emit to bus.target() mock return objects. |
| src/utils/bus-events/index.ts | Refactors getAccessToken() emits (formatting/chain style) but keeps dual-emits behavior. |
| src/core/internal/skyflow-frame/skyflow-frame-controller.ts | Targets Framebus emits to #clientDomain for controller readiness/frame-ready events. |
| src/core/internal/frame-element-init.ts | Targets Framebus COMPOSABLE_CONTAINER emit and adds origin validation for inbound message events. |
| src/core/internal/composable-frame-element-init.ts | Adds origin validation for inbound composable reveal/height message events. |
| src/core/external/skyflow-container.ts | Targets Framebus PureJS requests to properties.IFRAME_SECURE_ORIGIN consistently. |
| src/core/external/reveal/reveal-element.ts | Targets Framebus reveal calls to properties.IFRAME_SECURE_ORIGIN. |
| src/core/external/reveal/reveal-container.ts | Targets Framebus reveal request emit to properties.IFRAME_SECURE_ORIGIN. |
| src/core/external/reveal/composable-reveal-container.ts | Adds origin checks for inbound message events from composable iframes. |
| src/core/external/collect/compose-collect-container.ts | Adds origin checks for inbound message events from composable iframes. |
| package.json | Bumps SDK version string with a -dev.<sha> suffix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bus | ||
| // .target(properties.IFRAME_SECURE_ORIGIN) | ||
| .emit(ELEMENT_EVENTS_TO_IFRAME.GET_BEARER_TOKEN + clientId, {}, | ||
| (data:any) => { | ||
| if (data?.error) { | ||
| reject(data.error); | ||
| } | ||
| resolve(data.authToken); | ||
| }); | ||
|
|
||
| bus.emit(ELEMENT_EVENTS_TO_IFRAME.GET_BEARER_TOKEN, {}, | ||
| (data:any) => { | ||
| if (data?.error) { | ||
| reject(data.error); | ||
| } | ||
| resolve(data.authToken); | ||
| }); | ||
| bus | ||
| // .target(properties.IFRAME_SECURE_ORIGIN) | ||
| .emit(ELEMENT_EVENTS_TO_IFRAME.GET_BEARER_TOKEN, {}, | ||
| (data:any) => { | ||
| if (data?.error) { | ||
| reject(data.error); | ||
| } | ||
| resolve(data.authToken); | ||
| }); |
There was a problem hiding this comment.
getAccessToken() emits both GET_BEARER_TOKEN + clientId and GET_BEARER_TOKEN unconditionally. If clientId is empty this will emit the same event twice; even when non-empty it triggers unnecessary cross-frame traffic. Consider emitting only one event (prefer the suffixed one when clientId is present) and target properties.IFRAME_SECURE_ORIGIN to match the listener in src/skyflow.ts.
| (data:any) => { | ||
| if (data?.error) { | ||
| reject(data.error); | ||
| } | ||
| resolve(data.authToken); | ||
| }); |
There was a problem hiding this comment.
In both callbacks, reject(data.error) is followed by resolve(data.authToken) without an early return/else. This is confusing and can mask mistakes (e.g., resolving undefined if the promise is already settled). Add return reject(...) or wrap resolve in an else branch.
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
No description provided.