SPARK-843107: Remediate 5 low security findings across widget-demo, token-input, and widget-meet - #1469
Draft
mkesavan13 wants to merge 1 commit into
Conversation
…token-input, and widget-meet Bounds guest JWT input before the Identity Broker, guards/bounds/redacts the dev-only ciscoSparkEvents buffer, switches the developer portal link to HTTPS with safe rel attributes, removes raw bearer-token cookie persistence, and stops logging call destinations/ids on the call-placement path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
COMPLETES SPARK-843107
This pull request addresses
[SecScan][react-widgets][Low] Remediate 5 unique security findings.
Five confirmed Low security findings in this repository's own workspace source
under
packages/node_modules/@webex/*:size/shape check.
window.ciscoSparkEventsbuffer that grew without bound andcaptured raw event details (including identity/message fields), initialized
unconditionally (including in production) via
index.html.http://with no saferel/targetattributes.accessToken/accessTokenTypevalues persisted toweakly-configured browser cookies (no
Secure/SameSite/expiry/cleanup).raw call id via
console.log.by making the following changes
widget-demo/src/components/demo-widget/sdk.js— added aMAX_GUEST_JWT_LENGTH(8192-char) bound plus non-empty/string checks increateSDKGuestInstance; the accepted-input path is unchanged so theIdentity Broker remains authoritative for JWT validation.
widget-demo/src/components/demo-widget/index.js— added a shared,development-only
captureDemoEventhelper (no-op whenprocess.env.NODE_ENV === 'production', capped at 50 entries, stripsidentity/message-shaped keys from
detail) and routed bothonEventpushsites through it; removed the raw
accessToken/accessTokenTypecookies.setwrites and the corresponding constructorcookies.getreads(all non-token preference cookies are untouched).
widget-demo/src/index.html— removed the unconditionalwindow.ciscoSparkEvents = []init script (the dev-mode helper lazilycreates the array; no production global buffer exists).
private-react-component-token-input/src/index.js— switched thedeveloper portal anchor to
https://developer.webex.comwithtarget="_blank" rel="noopener noreferrer".widget-meet/src/enhancers/withCallHandlers.js— removed the twoconsole.logstatements that disclosed the meeting SIP/webLink destinationand the placed-call id; call placement and
storeMeetDetails({callId})arepreserved. Exported
handleCall(previously module-private) so it isdirectly unit-testable.
*.test.jssuites (9 tests total) covering allfive findings — see Testing below.
Change Type
The following scenarios were tested
New targeted unit tests were run during implementation (RED → GREEN → targeted
regression per task) with a final combined run of all four new test files: 4
suites, 9 tests, all passing at the time of the Fix step. This delivery step
does not rerun those tests; see
## Testingbelow for the recorded evidenceand gate status.
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Testing
*.test.jsfiles:sdk.test.js3,demo-widget/index.test.js4,token-input/index.test.js1,withCallHandlers.test.js1).skipped-missing-command); this PR has not been verified by the repository's compile/unit-test gates and must not be merged without independent human verification of the recorded test evidence below.Recorded evidence from the earlier Fix step (targeted per-task RED/GREEN/regression
runs via
npx jest --config jest.config.json <file>, not rerun during delivery):sdk.test.jsindex.test.js(ciscoSparkEvents capture)private-react-component-token-input/src/index.test.jsindex.test.js(token cookie storage)withCallHandlers.test.jsAcceptance Criteria
sdk.test.js3/3 passing (recorded Fix-step evidence; Gate 2 not run:skipped-missing-command)window.ciscoSparkEventsbuffer; any event capture is development-only, bounded, and redacted of message/identity metadata.index.test.js(ciscoSparkEvents capture) 2/2 passing (recorded Fix-step evidence; Gate 2 not run:skipped-missing-command)private-react-component-token-input/src/index.test.js1/1 passing (recorded Fix-step evidence; Gate 2 not run:skipped-missing-command)SecureandSameSite, minimizes lifetime/scope, and cleans up.index.test.js(token cookie storage) 2/2 passing (recorded Fix-step evidence; Gate 2 not run:skipped-missing-command). Demo-storage owner sign-off is still pending — see External Validation Required below.withCallHandlers.test.js1/1 passing (recorded Fix-step evidence; Gate 2 not run:skipped-missing-command)External Validation Required
owner-sign-offaccessToken/accessTokenTypecookie writes and reads indemo-widget/index.js, at what were lines 101–102 and 127–128 pre-fix) changes how demo user browser storage persists bearer tokens. The code-level removal is implemented and unit-tested (index.test.js→token cookie storage), but per the implementation plan's boundary-impact-4 note, the demo-storage owner must externally confirm this token-persistence removal is acceptable before delivery. If external validation instead prefers retaining demo-only storage, the equivalent remediation is to setSecure+SameSite, a minimized expiry, and explicit cleanup on those cookies.External validation for AC-4 remains explicitly unrun by JiraToPr; unit evidence
is reported above while the demo-storage owner's sign-off remains pending for
human review.
Contract Discovery Warnings
.sdd/manifest.json,AGENTS.md, or spec index governs the affected demo/widget files;target/affected-package-manifests.jsonlists no package manifests for these paths. Planning relied on repository-inference from the affected files.AI Assistance
Jira: https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-843107