-
Notifications
You must be signed in to change notification settings - Fork 70
fix(cc-widgets): remediate P0 security scan findings #728
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
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,31 +12,6 @@ concurrency: | |
|
|
||
| env: | ||
| rid: ${{ github.run_id }}-${{ github.run_number }} | ||
| PW_SANDBOX_PASSWORD: ${{ secrets.PW_SANDBOX_PASSWORD }} | ||
| PW_SENDER_EMAIL_PASSWORD: ${{ secrets.PW_SENDER_EMAIL_PASSWORD }} | ||
| PW_SANDBOX: ${{ secrets.PW_SANDBOX }} | ||
| PW_ENTRY_POINT1: ${{ secrets.PW_ENTRY_POINT1 }} | ||
| PW_ENTRY_POINT2: ${{ secrets.PW_ENTRY_POINT2 }} | ||
| PW_ENTRY_POINT3: ${{ secrets.PW_ENTRY_POINT3 }} | ||
| PW_ENTRY_POINT4: ${{ secrets.PW_ENTRY_POINT4 }} | ||
| PW_ENTRY_POINT5: ${{ secrets.PW_ENTRY_POINT5 }} | ||
| PW_ENTRY_POINT6: ${{ secrets.PW_ENTRY_POINT6 }} | ||
| PW_ENTRY_POINT7: ${{ secrets.PW_ENTRY_POINT7 }} | ||
| PW_ENTRY_POINT8: ${{ secrets.PW_ENTRY_POINT8 }} | ||
| PW_ENTRY_POINT9: ${{ secrets.PW_ENTRY_POINT9 }} | ||
| PW_ENTRY_POINT10: ${{ secrets.PW_ENTRY_POINT10 }} | ||
| PW_ENTRY_POINT11: ${{ secrets.PW_ENTRY_POINT11 }} | ||
| PW_ENTRY_POINT12: ${{ secrets.PW_ENTRY_POINT12 }} | ||
| PW_SENDER_EMAIL: ${{ secrets.PW_SENDER_EMAIL }} | ||
| PW_CHAT_URL: ${{ secrets.PW_CHAT_URL }} | ||
| PW_DIAL_NUMBER_LOGIN_USERNAME: ${{ secrets.PW_DIAL_NUMBER_LOGIN_USERNAME }} | ||
| PW_DIAL_NUMBER_LOGIN_PASSWORD: ${{ secrets.PW_DIAL_NUMBER_LOGIN_PASSWORD }} | ||
| PW_DIAL_NUMBER: ${{ secrets.PW_DIAL_NUMBER }} | ||
| PW_DIAL_NUMBER_NAME: ${{ secrets.PW_DIAL_NUMBER_NAME }} | ||
| PW_ENTRYPOINT_NAME: ${{ secrets.PW_ENTRYPOINT_NAME }} | ||
| PW_MEETING_USERID: ${{ secrets.PW_MEETING_USERID }} | ||
| PW_MEETING_PASSWORD: ${{ secrets.PW_MEETING_PASSWORD }} | ||
| PW_MEETING_DESTINATION: ${{ secrets.PW_MEETING_DESTINATION}} | ||
|
|
||
| jobs: | ||
| validate: | ||
|
|
@@ -140,6 +115,32 @@ jobs: | |
| concurrency: | ||
| group: e2e-meetings | ||
| cancel-in-progress: false | ||
| env: | ||
| PW_SANDBOX_PASSWORD: ${{ secrets.PW_SANDBOX_PASSWORD }} | ||
|
Comment on lines
+118
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a fork PR receives both the Useful? React with 👍 / 👎. |
||
| PW_SENDER_EMAIL_PASSWORD: ${{ secrets.PW_SENDER_EMAIL_PASSWORD }} | ||
| PW_SANDBOX: ${{ secrets.PW_SANDBOX }} | ||
| PW_ENTRY_POINT1: ${{ secrets.PW_ENTRY_POINT1 }} | ||
| PW_ENTRY_POINT2: ${{ secrets.PW_ENTRY_POINT2 }} | ||
| PW_ENTRY_POINT3: ${{ secrets.PW_ENTRY_POINT3 }} | ||
| PW_ENTRY_POINT4: ${{ secrets.PW_ENTRY_POINT4 }} | ||
| PW_ENTRY_POINT5: ${{ secrets.PW_ENTRY_POINT5 }} | ||
| PW_ENTRY_POINT6: ${{ secrets.PW_ENTRY_POINT6 }} | ||
| PW_ENTRY_POINT7: ${{ secrets.PW_ENTRY_POINT7 }} | ||
| PW_ENTRY_POINT8: ${{ secrets.PW_ENTRY_POINT8 }} | ||
| PW_ENTRY_POINT9: ${{ secrets.PW_ENTRY_POINT9 }} | ||
| PW_ENTRY_POINT10: ${{ secrets.PW_ENTRY_POINT10 }} | ||
| PW_ENTRY_POINT11: ${{ secrets.PW_ENTRY_POINT11 }} | ||
| PW_ENTRY_POINT12: ${{ secrets.PW_ENTRY_POINT12 }} | ||
| PW_SENDER_EMAIL: ${{ secrets.PW_SENDER_EMAIL }} | ||
| PW_CHAT_URL: ${{ secrets.PW_CHAT_URL }} | ||
| PW_DIAL_NUMBER_LOGIN_USERNAME: ${{ secrets.PW_DIAL_NUMBER_LOGIN_USERNAME }} | ||
| PW_DIAL_NUMBER_LOGIN_PASSWORD: ${{ secrets.PW_DIAL_NUMBER_LOGIN_PASSWORD }} | ||
| PW_DIAL_NUMBER: ${{ secrets.PW_DIAL_NUMBER }} | ||
| PW_DIAL_NUMBER_NAME: ${{ secrets.PW_DIAL_NUMBER_NAME }} | ||
| PW_ENTRYPOINT_NAME: ${{ secrets.PW_ENTRYPOINT_NAME }} | ||
| PW_MEETING_USERID: ${{ secrets.PW_MEETING_USERID }} | ||
| PW_MEETING_PASSWORD: ${{ secrets.PW_MEETING_PASSWORD }} | ||
| PW_MEETING_DESTINATION: ${{ secrets.PW_MEETING_DESTINATION }} | ||
|
|
||
| steps: | ||
| - name: Checkout Project | ||
|
|
@@ -194,6 +195,32 @@ jobs: | |
| concurrency: | ||
| group: e2e-cc-widgets | ||
| cancel-in-progress: false | ||
| env: | ||
| PW_SANDBOX_PASSWORD: ${{ secrets.PW_SANDBOX_PASSWORD }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can have a common file from where these are exported and that can be used here in PR pipeline |
||
| PW_SENDER_EMAIL_PASSWORD: ${{ secrets.PW_SENDER_EMAIL_PASSWORD }} | ||
| PW_SANDBOX: ${{ secrets.PW_SANDBOX }} | ||
| PW_ENTRY_POINT1: ${{ secrets.PW_ENTRY_POINT1 }} | ||
| PW_ENTRY_POINT2: ${{ secrets.PW_ENTRY_POINT2 }} | ||
| PW_ENTRY_POINT3: ${{ secrets.PW_ENTRY_POINT3 }} | ||
| PW_ENTRY_POINT4: ${{ secrets.PW_ENTRY_POINT4 }} | ||
| PW_ENTRY_POINT5: ${{ secrets.PW_ENTRY_POINT5 }} | ||
| PW_ENTRY_POINT6: ${{ secrets.PW_ENTRY_POINT6 }} | ||
| PW_ENTRY_POINT7: ${{ secrets.PW_ENTRY_POINT7 }} | ||
| PW_ENTRY_POINT8: ${{ secrets.PW_ENTRY_POINT8 }} | ||
| PW_ENTRY_POINT9: ${{ secrets.PW_ENTRY_POINT9 }} | ||
| PW_ENTRY_POINT10: ${{ secrets.PW_ENTRY_POINT10 }} | ||
| PW_ENTRY_POINT11: ${{ secrets.PW_ENTRY_POINT11 }} | ||
| PW_ENTRY_POINT12: ${{ secrets.PW_ENTRY_POINT12 }} | ||
| PW_SENDER_EMAIL: ${{ secrets.PW_SENDER_EMAIL }} | ||
| PW_CHAT_URL: ${{ secrets.PW_CHAT_URL }} | ||
| PW_DIAL_NUMBER_LOGIN_USERNAME: ${{ secrets.PW_DIAL_NUMBER_LOGIN_USERNAME }} | ||
| PW_DIAL_NUMBER_LOGIN_PASSWORD: ${{ secrets.PW_DIAL_NUMBER_LOGIN_PASSWORD }} | ||
| PW_DIAL_NUMBER: ${{ secrets.PW_DIAL_NUMBER }} | ||
| PW_DIAL_NUMBER_NAME: ${{ secrets.PW_DIAL_NUMBER_NAME }} | ||
| PW_ENTRYPOINT_NAME: ${{ secrets.PW_ENTRYPOINT_NAME }} | ||
| PW_MEETING_USERID: ${{ secrets.PW_MEETING_USERID }} | ||
| PW_MEETING_PASSWORD: ${{ secrets.PW_MEETING_PASSWORD }} | ||
| PW_MEETING_DESTINATION: ${{ secrets.PW_MEETING_DESTINATION }} | ||
|
|
||
| steps: | ||
| - name: Checkout Project | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| const {execSync} = require('child_process'); | ||
| const {execFileSync} = require('child_process'); | ||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
|
|
@@ -42,6 +42,17 @@ function versionAndPublish() { | |
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Validate branchName (used as npm dist-tag): must match npm tag naming rules. | ||
| // Reject any value containing shell metacharacters or characters outside the allowed set. | ||
| const validTagPattern = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; | ||
| if (!validTagPattern.test(branchName)) { | ||
| console.error( | ||
| `Error: Invalid branchName/tag value "${branchName}". Must match /^[A-Za-z0-9][A-Za-z0-9._-]*$/` | ||
| ); | ||
| process.exit(1); | ||
| return; | ||
| } | ||
| const contactCenterPath = './packages/contact-center'; | ||
|
|
||
| try { | ||
|
|
@@ -61,10 +72,16 @@ function versionAndPublish() { | |
| return packageData.name; | ||
| }); | ||
|
|
||
| // Publish the package | ||
| // Validate workspace names (npm package name pattern) and publish via no-shell execFileSync. | ||
| // Using execFileSync instead of execSync prevents shell interpretation of workspace or tag values. | ||
| const validPackageNamePattern = /^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/; | ||
|
|
||
| const publishWorkspace = (workspace) => { | ||
| if (!validPackageNamePattern.test(workspace)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we validating this in 2 places ? Validating here should take care of it right |
||
| throw new Error(`Invalid package name "${workspace}": does not match npm package name pattern`); | ||
| } | ||
| console.log(`Publishing new version for ${workspace}: ${newVersion}`); | ||
| execSync(`yarn workspace ${workspace} npm publish --tag ${branchName}`, {stdio: 'inherit'}); | ||
| execFileSync('yarn', ['workspace', workspace, 'npm', 'publish', '--tag', branchName], {stdio: 'inherit'}); | ||
| }; | ||
|
|
||
| const denyList = ['@webex/test-fixtures']; // Add workspace names to exclude from publishing | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,6 @@ import './App.scss'; | |
| import {observer} from 'mobx-react-lite'; | ||
| import EngageWidget from './EngageWidget'; | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| // This is not to be included to a production app. | ||
| // Have added here for debugging purposes | ||
| window['store'] = store; | ||
|
|
||
| const defaultWidgets = { | ||
| stationLogin: true, | ||
| stationLoginProfile: false, | ||
|
|
@@ -131,22 +127,19 @@ function App() { | |
| }; | ||
|
|
||
| useEffect(() => { | ||
| // Remove token persisted by older sample builds — no longer read or written (SPARK-833336) | ||
| window.localStorage.removeItem('accessToken'); | ||
|
|
||
| if (window.location.hash) { | ||
| const urlParams = new URLSearchParams(window.location.hash.replace('#', '?')); | ||
|
|
||
| const accessToken = urlParams.get('access_token'); | ||
|
|
||
| if (accessToken) { | ||
| window.localStorage.setItem('accessToken', accessToken); | ||
| setAccessToken(accessToken); | ||
| // Clear the hash from the URL to remove the token from browser history | ||
| window.history.replaceState({}, document.title, window.location.pathname + window.location.search); | ||
| } | ||
| } else { | ||
| const storedAccessToken = window.localStorage.getItem('accessToken'); | ||
| if (storedAccessToken) { | ||
| setAccessToken(storedAccessToken); | ||
| } | ||
| } | ||
| }, []); | ||
|
|
||
|
|
@@ -379,13 +372,6 @@ function App() { | |
| }); | ||
| }; | ||
|
|
||
| // Store accessToken changes in local storage | ||
| useEffect(() => { | ||
| if (accessToken.trim() !== '') { | ||
| window.localStorage.setItem('accessToken', accessToken); | ||
| } | ||
| }, [accessToken]); | ||
|
|
||
| useEffect(() => { | ||
| window.localStorage.setItem('selectedWidgets', JSON.stringify(selectedWidgets)); | ||
| }, [selectedWidgets]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ | |
| } | ||
| } | ||
|
|
||
| function toggleintegrationEnv() { | ||
| integrationEnv = integrationEnvElem.checked; | ||
| // Store in localStorage for persistence | ||
| localStorage.setItem('integrationEnv', JSON.stringify(integrationEnv)); | ||
|
|
@@ -82,12 +82,12 @@ | |
| } | ||
| } | ||
|
|
||
| function enableMultiLogin() { | ||
| if (isMultiLoginEnabled) isMultiLoginEnabled = false; | ||
| else isMultiLoginEnabled = true; | ||
| } | ||
|
|
||
| function toggleCampaignPreview(event) { | ||
| hasCampaignPreviewEnabled = event.target.checked; | ||
| // Sync both checkboxes | ||
| document.querySelectorAll('.campaign-preview-checkbox').forEach(function (cb) { | ||
|
|
@@ -96,7 +96,7 @@ | |
| ccTaskList.hasCampaignPreviewEnabled = hasCampaignPreviewEnabled; | ||
| } | ||
|
|
||
| function doOAuthLogin() { | ||
| let redirectUri = `${window.location.protocol}//${window.location.host}`; | ||
|
|
||
| if (window.location.pathname) { | ||
|
|
@@ -135,7 +135,7 @@ | |
| }, | ||
| }; | ||
|
|
||
| const webex = Webex.init(webexConfig); | ||
|
|
||
| webex.once('ready', () => { | ||
| webex.authorization.initiateLogin(); | ||
|
|
@@ -150,6 +150,9 @@ | |
| accessTokenElem.addEventListener('keyup', updateButtonState); | ||
|
|
||
| window.addEventListener('load', () => { | ||
| // Remove token persisted by older sample builds — no longer read or written (SPARK-833336) | ||
| localStorage.removeItem('accessToken'); | ||
|
|
||
| changeLoginType(); | ||
| loadintegrationEnvSetting(); // Load the setting on page load | ||
| if (window.location.hash) { | ||
|
|
@@ -158,22 +161,15 @@ | |
| const accessToken = urlParams.get('access_token'); | ||
|
|
||
| if (accessToken) { | ||
| localStorage.setItem('accessToken', accessToken); | ||
| accessTokenElem.value = accessToken; | ||
| updateButtonState(); | ||
| // Clear the hash from the URL to remove the token from browser history | ||
| window.history.replaceState({}, document.title, window.location.pathname + window.location.search); | ||
| } | ||
| } else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How are we verifying if this change doesn't break anything ? |
||
| const storedAccessToken = window.localStorage.getItem('accessToken'); | ||
| if (storedAccessToken) { | ||
| accessTokenElem.value = storedAccessToken; | ||
| updateButtonState(); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| function initWidgets() { | ||
| const webexConfig = { | ||
| fedramp: false, | ||
| logger: { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit changes credential persistence, CI secret scoping, and the publish command's trust boundary without updating any module spec or standing document. That leaves the repository's security design and verification guidance out of sync with the implementation despite the mandatory same-change spec-currency rule; update the relevant standing security documentation alongside these controls.
AGENTS.md reference: AGENTS.md:L68-L68
Useful? React with 👍 / 👎.