Conversation
|
Important Review skippedToo many files! 25 files out of 175 files are above the max files limit of 150. You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| runs-on: ubuntu-latest | ||
| if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }} | ||
| steps: | ||
| - name: Skip CI check | ||
| run: echo "Proceeding with workflow" | ||
|
|
||
| test: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the fix is to explicitly declare permissions for the workflow (or per job) so that the GITHUB_TOKEN does not silently inherit overly broad repository defaults. Start from a minimal read-only configuration and only add more granular write permissions where a job truly needs them.
The best single change here, without altering existing functionality, is to add a permissions block at the top (root) level of .github/workflows/react-native-cicd.yml, just below the on: section (and before env:). From the visible snippet, none of the shown jobs push commits, create releases, modify issues/PRs, or otherwise write back to the repository via GITHUB_TOKEN; they mainly use checkout, caching, Node setup, dependency install, tests, and likely builds/deploys that rely on external secrets. All of these operations work with contents: read. Therefore, we can safely set:
permissions:
contents: readat the workflow root. This will apply to all jobs (check-skip, test, build-and-deploy, etc.) that do not override permissions. If any hidden portion of the workflow later requires write access to specific resources, an additional permissions block can be added for that specific job with the needed granular write scopes. No imports or extra definitions are required; this is purely a YAML configuration change.
| @@ -35,6 +35,9 @@ | ||
| description: 'Manual release notes override' | ||
| required: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| env: | ||
| EXPO_TOKEN: ${{ secrets.EXPO_TOKEN }} | ||
| EXPO_APPLE_ID: ${{ secrets.EXPO_APPLE_ID }} |
| needs: check-skip | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: 🏗 Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: 🏗 Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '24' | ||
| cache: 'yarn' | ||
|
|
||
| - name: 📦 Setup yarn cache | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: | | ||
| ~/.cache/yarn | ||
| node_modules | ||
| key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-yarn- | ||
|
|
||
| - name: 📦 Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
|
|
||
| - name: 🧪 Run Checks and Tests | ||
| run: yarn check-all | ||
|
|
||
| build-and-deploy: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the fix is to explicitly set permissions: for the workflow or for each job so that the GITHUB_TOKEN only has the minimum scopes needed. For the test job (and the other jobs shown), the steps only read repository contents (via actions/checkout) and use external services/secrets; they do not need to write to the repo or to PRs. The simplest and safest choice is to define a workflow-level permissions: contents: read, which applies to all jobs (check-skip, test, build-and-deploy) and satisfies CodeQL’s recommendation.
Concretely, in .github/workflows/react-native-cicd.yml, add a root-level permissions: block right after the name: (line 1–2 area) and before the on: block starting at line 3. Set contents: read as a minimal default; if any job later needs more (e.g., pull-requests: write), it can override permissions at the job level without affecting this fix. No additional imports or methods are required because this is a YAML configuration change only.
| @@ -1,5 +1,8 @@ | ||
| name: React Native CI/CD | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] |
| needs: test | ||
| if: (github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master')) || github.event_name == 'workflow_dispatch' | ||
| strategy: | ||
| matrix: | ||
| platform: [android, ios] | ||
| runs-on: ${{ matrix.platform == 'ios' && 'macos-15' || 'ubuntu-latest' }} | ||
| environment: RNBuild | ||
| steps: | ||
| - name: 🏗 Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: 🏗 Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '24' | ||
| cache: 'yarn' | ||
|
|
||
| - name: 🔎 Verify Xcode toolchain | ||
| if: matrix.platform == 'ios' | ||
| run: | | ||
| xcodebuild -version | ||
| swift --version | ||
|
|
||
| - name: 🏗 Setup Expo | ||
| uses: expo/expo-github-action@v8 | ||
| with: | ||
| expo-version: latest | ||
| eas-version: latest | ||
| token: ${{ secrets.EXPO_TOKEN }} | ||
|
|
||
| - name: 📦 Setup yarn cache | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: | | ||
| ~/.cache/yarn | ||
| node_modules | ||
| key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-yarn- | ||
|
|
||
| - name: 📦 Install dependencies | ||
| run: | | ||
| yarn install --frozen-lockfile | ||
|
|
||
| - name: 📋 Create Google Json File | ||
| run: | | ||
| echo $RESPOND_GOOGLE_SERVICES | base64 -d > google-services.json | ||
|
|
||
| - name: 📋 Update package.json Versions | ||
| run: | | ||
| # Ensure jq is available on both Linux and macOS | ||
| if ! command -v jq &> /dev/null; then | ||
| echo "Installing jq..." | ||
| if [ "${RUNNER_OS}" = "Linux" ]; then | ||
| sudo apt-get update && sudo apt-get install -y jq | ||
| elif [ "${RUNNER_OS}" = "macOS" ]; then | ||
| brew update || true | ||
| brew install jq | ||
| else | ||
| echo "Unsupported runner OS: ${RUNNER_OS}" >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| androidVersionCode=$((5080345 + ${{ github.run_number }})) | ||
| echo "Android Version Code: ${androidVersionCode}" | ||
|
|
||
| # Fix the main entry in package.json | ||
| if [ -f ./package.json ]; then | ||
| # Create a backup | ||
| cp package.json package.json.bak | ||
| # Update the package.json | ||
| jq --arg version "10.${{ github.run_number }}" --argjson versionCode "$androidVersionCode" '.version = $version | .versionCode = $versionCode' package.json > package.json.tmp && mv package.json.tmp package.json | ||
| echo "Updated package.json versions" | ||
| cat package.json | grep "version" | ||
| cat package.json | grep "versionCode" | ||
| else | ||
| echo "package.json not found" | ||
| exit 1 | ||
| fi | ||
|
|
||
| - name: 📱 Setup EAS build cache | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: ~/.eas-build-local | ||
| key: ${{ runner.os }}-eas-build-local-${{ hashFiles('**/package.json') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-eas-build-local- | ||
|
|
||
| - name: 🔄 Verify EAS CLI installation | ||
| run: | | ||
| echo "EAS CLI version:" | ||
| eas --version | ||
|
|
||
| - name: 📋 Create iOS Cert | ||
| run: | | ||
| echo $RESPOND_IOS_CERT | base64 -d > AuthKey_HRBP5FNJN6.p8 | ||
|
|
||
| - name: 📋 Restore gradle.properties | ||
| env: | ||
| GRADLE_PROPERTIES: ${{ secrets.GRADLE_PROPERTIES }} | ||
| shell: bash | ||
| run: | | ||
| mkdir -p ~/.gradle/ | ||
| echo ${GRADLE_PROPERTIES} > ~/.gradle/gradle.properties | ||
|
|
||
| - name: 📱 Build Development APK | ||
| if: (matrix.platform == 'android' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'dev')) | ||
| run: | | ||
| # Build with increased memory limit | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform android --profile development --local --non-interactive --output=./ResgridRespond-dev.apk | ||
| env: | ||
| NODE_ENV: development | ||
|
|
||
| - name: 📱 Build Production APK | ||
| if: (matrix.platform == 'android' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'prod-apk')) | ||
| run: | | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform android --profile production-apk --local --non-interactive --output=./ResgridRespond-prod.apk | ||
| env: | ||
| NODE_ENV: production | ||
|
|
||
| - name: 📱 Build Production AAB | ||
| if: (matrix.platform == 'android' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'prod-aab')) | ||
| run: | | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform android --profile production --local --non-interactive --output=./ResgridRespond-prod.aab | ||
| env: | ||
| NODE_ENV: production | ||
|
|
||
| - name: 📱 Build iOS Development | ||
| if: (matrix.platform == 'ios' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'ios-dev')) | ||
| run: | | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform ios --profile development --local --non-interactive --output=./ResgridRespond-ios-dev.ipa | ||
| env: | ||
| NODE_ENV: development | ||
|
|
||
| - name: 📱 Build iOS Ad-Hoc | ||
| if: (matrix.platform == 'ios' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'ios-adhoc')) | ||
| run: | | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform ios --profile internal --local --non-interactive --output=./ResgridRespond-ios-adhoc.ipa | ||
| env: | ||
| NODE_ENV: production | ||
|
|
||
| - name: 📱 Build iOS Production | ||
| if: (matrix.platform == 'ios' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'ios-prod')) | ||
| run: | | ||
| export NODE_OPTIONS="--openssl-legacy-provider --max_old_space_size=4096" | ||
| eas build --platform ios --profile production --local --non-interactive --output=./ResgridRespond-ios-prod.ipa | ||
| env: | ||
| NODE_ENV: production | ||
|
|
||
| - name: 📦 Upload build artifacts to GitHub | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: app-builds-${{ matrix.platform }} | ||
| path: | | ||
| ./ResgridRespond-dev.apk | ||
| ./ResgridRespond-prod.apk | ||
| ./ResgridRespond-prod.aab | ||
| ./ResgridRespond-ios-dev.ipa | ||
| ./ResgridRespond-ios-adhoc.ipa | ||
| ./ResgridRespond-ios-prod.ipa | ||
| retention-days: 7 | ||
|
|
||
| - name: 📦 Setup Firebase CLI | ||
| uses: w9jds/setup-firebase@main | ||
| with: | ||
| tools-version: 11.9.0 | ||
| firebase_token: ${{ secrets.FIREBASE_TOKEN }} | ||
|
|
||
| - name: 📦 Upload Android artifact to Firebase App Distribution | ||
| if: (matrix.platform == 'android') | ||
| run: | | ||
| firebase appdistribution:distribute ./ResgridRespond-prod.apk --app ${{ secrets.FIREBASE_RESP_ANDROID_APP_ID }} --groups "testers" | ||
|
|
||
| - name: 📦 Upload iOS artifact to Firebase App Distribution | ||
| if: (matrix.platform == 'ios') | ||
| run: | | ||
| firebase appdistribution:distribute ./ResgridRespond-ios-adhoc.ipa --app ${{ secrets.FIREBASE_RESP_IOS_APP_ID }} --groups "testers" | ||
|
|
||
| - name: 📋 Prepare Release Notes file | ||
| if: ${{ matrix.platform == 'android' }} | ||
| env: | ||
| RELEASE_NOTES_INPUT: ${{ github.event.inputs.release_notes }} | ||
| PR_BODY: ${{ github.event.pull_request.body }} | ||
| run: | | ||
| set -eo pipefail | ||
| # Determine source of release notes: workflow input, PR body, or recent commits | ||
| if [ -n "$RELEASE_NOTES_INPUT" ]; then | ||
| NOTES="$RELEASE_NOTES_INPUT" | ||
| elif [ -n "$PR_BODY" ]; then | ||
| NOTES="$(printf '%s\n' "$PR_BODY" \ | ||
| | awk 'f && /^## /{exit} /^## Release Notes/{f=1; next} f')" | ||
| else | ||
| NOTES="$(git log -n 5 --pretty=format:'- %s')" | ||
| fi | ||
| # Fail if no notes extracted | ||
| if [ -z "$NOTES" ]; then | ||
| echo "Error: No release notes extracted" >&2 | ||
| exit 1 | ||
| fi | ||
| # Write header and notes to file | ||
| { | ||
| echo "## Version 10.${{ github.run_number }} - $(date +%Y-%m-%d)" | ||
| echo | ||
| printf '%s\n' "$NOTES" | ||
| } > RELEASE_NOTES.md | ||
|
|
||
| - name: 📦 Create Release | ||
| if: ${{ matrix.platform == 'android' && (github.event.inputs.buildType == 'all' || github.event_name == 'push' || github.event.inputs.buildType == 'prod-apk') }} | ||
| uses: ncipollo/release-action@v1 | ||
| with: | ||
| tag: '10.${{ github.run_number }}' | ||
| commit: ${{ github.sha }} | ||
| makeLatest: true | ||
| allowUpdates: true | ||
| name: '10.${{ github.run_number }}' | ||
| artifacts: './ResgridRespond-prod.apk' | ||
| bodyFile: 'RELEASE_NOTES.md' No newline at end of file |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the fix is to explicitly define permissions: for the workflow or for individual jobs so that the GITHUB_TOKEN is scoped to the least privileges required. For CI-only jobs, contents: read is usually enough. For jobs that create releases or modify repository content, we add targeted write permissions like contents: write.
For this workflow, the simplest non-breaking change is to add a top-level permissions: block that applies to all jobs, setting contents: write. The build-and-deploy job needs contents: write so that ncipollo/release-action@v1 can create or update GitHub Releases. The check-skip and test jobs don’t need write access, but inheriting contents: write from the root is acceptable and keeps the edit small and clear. We will insert the permissions: block near the top of the file, after name: and before on:. No extra imports or dependencies are needed; this is purely a YAML configuration change.
Concretely, in .github/workflows/react-native-cicd.yml, between line 1 (name: React Native CI/CD) and line 3 (on:), we will add:
permissions:
contents: writeThis explicitly narrows the GITHUB_TOKEN to repository contents only, instead of potentially broader default permissions, and provides the write access required for creating releases.
| @@ -1,5 +1,8 @@ | ||
| name: React Native CI/CD | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] |
| // Extract HTML content from source.html for testing | ||
| const htmlContent = props.source?.html || ''; | ||
| // Simple extraction of text content from HTML (removing tags) | ||
| const textContent = htmlContent.replace(/<[^>]*>/g, '').trim(); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the fix is to avoid implementing ad‑hoc HTML “sanitization” with a single fragile regex that removes whole tags. Instead, either (a) use a real HTML parser / sanitization library, or (b) if we just need text content, parse HTML into a DOM-like structure and extract text nodes. This avoids the multi-character replacement issue (where replacing a long pattern lets a new instance of the pattern appear) and avoids the common pitfalls of regex-based HTML parsing.
For this specific test file, the intent is only to “extract text content from HTML (removing tags)” to feed into a Text component. We can most closely preserve existing behavior by using a small, well-known HTML parsing library to convert htmlContent to plain text. A simple and robust option is html-to-text, which is widely used to turn HTML into readable text. We will:
- Add an import for
html-to-textat the top of this test file. - Replace the line using
htmlContent.replace(/<[^>]*>/g, '').trim();with a call tohtmlToTextthat converts the HTML into text, and trims it.
Concretely, in src/components/contacts/__tests__/contact-notes-list.test.tsx:
- Add
import { htmlToText } from 'html-to-text';after the existing imports. - Change line 27 to:
const textContent = htmlToText(htmlContent).trim();
This removes the fragile regex, uses a library designed for this transformation, and keeps the rest of the mock and tests unchanged.
| @@ -1,6 +1,7 @@ | ||
| import { render } from '@testing-library/react-native'; | ||
| import { beforeEach, describe, expect, it, jest } from '@jest/globals'; | ||
| import React from 'react'; | ||
| import { htmlToText } from 'html-to-text'; | ||
|
|
||
| import { ContactNotesList } from '../contact-notes-list'; | ||
|
|
||
| @@ -23,8 +24,8 @@ | ||
| default: React.forwardRef((props: any, ref: any) => { | ||
| // Extract HTML content from source.html for testing | ||
| const htmlContent = props.source?.html || ''; | ||
| // Simple extraction of text content from HTML (removing tags) | ||
| const textContent = htmlContent.replace(/<[^>]*>/g, '').trim(); | ||
| // Extract text content from HTML using a robust HTML-to-text converter | ||
| const textContent = htmlToText(htmlContent).trim(); | ||
|
|
||
| return React.createElement(View, { | ||
| ...props, |
| @@ -174,7 +174,8 @@ | ||
| "react-query-kit": "~3.3.0", | ||
| "tailwind-variants": "~0.2.1", | ||
| "zod": "~3.23.8", | ||
| "zustand": "~4.5.5" | ||
| "zustand": "~4.5.5", | ||
| "html-to-text": "^9.0.5" | ||
| }, | ||
| "devDependencies": { | ||
| "@babel/core": "~7.26.0", |
| Package | Version | Security advisories |
| html-to-text (npm) | 9.0.5 | None |
| jest.mock('@/lib/utils', () => ({ | ||
| formatDateForDisplay: jest.fn((date) => date ? '2023-01-01 12:00 UTC' : ''), | ||
| parseDateISOString: jest.fn((dateString) => dateString ? new Date(dateString) : null), | ||
| stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''), |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the problem is that /\<[^>]*>/g is a simplistic, custom HTML-stripper regex that can leave unsafe substrings (like parts of <script> tags) in the string and is susceptible to incomplete multi-character sanitization. The recommended approach is to avoid rolling a custom sanitizer and instead rely on a well-tested HTML sanitization/stripping library, or at least use a more robust pattern that better approximates what production code should do.
For this specific file, the best fix that does not change the intended test functionality is to update the Jest mock of stripHtmlTags to use a library that properly strips/sanitizes HTML, while keeping the same signature and behavior for non-string inputs. Since we can only modify code in this file, we can introduce an import of a known library such as sanitize-html and then implement stripHtmlTags as a thin wrapper around it, returning an empty string for falsy input (to preserve existing behavior). This addresses CodeQL’s “incomplete multi-character sanitization” complaint and makes the test mock more realistic. Concretely:
- Add
import sanitizeHtml from 'sanitize-html';at the top ofsrc/components/protocols/__tests__/protocol-card.test.tsxalongside the existing imports. - Change the
stripHtmlTagsmock implementation from:stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''),
to:stripHtmlTags: jest.fn((html) => html ? sanitizeHtml(html, { allowedTags: [], allowedAttributes: {} }) : ''),
- This keeps the same external interface while using a robust library internally.
| @@ -1,5 +1,6 @@ | ||
| import { render, screen, fireEvent } from '@testing-library/react-native'; | ||
| import React from 'react'; | ||
| import sanitizeHtml from 'sanitize-html'; | ||
|
|
||
| import { CallProtocolsResultData } from '@/models/v4/callProtocols/callProtocolsResultData'; | ||
|
|
||
| @@ -9,7 +10,7 @@ | ||
| jest.mock('@/lib/utils', () => ({ | ||
| formatDateForDisplay: jest.fn((date) => date ? '2023-01-01 12:00 UTC' : ''), | ||
| parseDateISOString: jest.fn((dateString) => dateString ? new Date(dateString) : null), | ||
| stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''), | ||
| stripHtmlTags: jest.fn((html) => html ? sanitizeHtml(html, { allowedTags: [], allowedAttributes: {} }) : ''), | ||
| })); | ||
|
|
||
| describe('ProtocolCard', () => { |
| @@ -174,7 +174,8 @@ | ||
| "react-query-kit": "~3.3.0", | ||
| "tailwind-variants": "~0.2.1", | ||
| "zod": "~3.23.8", | ||
| "zustand": "~4.5.5" | ||
| "zustand": "~4.5.5", | ||
| "sanitize-html": "^2.17.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@babel/core": "~7.26.0", |
| Package | Version | Security advisories |
| sanitize-html (npm) | 2.17.0 | None |
| jest.mock('@/lib/utils', () => ({ | ||
| formatDateForDisplay: jest.fn((date) => date ? '2023-01-01 12:00 UTC' : ''), | ||
| parseDateISOString: jest.fn((dateString) => dateString ? new Date(dateString) : null), | ||
| stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''), |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the right fix is to avoid hand‑written, brittle regex‑based HTML “sanitizers” and instead rely on a well‑tested library that correctly handles nested tags, edge cases, and multi‑character replacement issues. In Node/JS, a common choice is sanitize-html. Even though this is test code, we can update the mock stripHtmlTags to delegate to sanitize-html so that tests better match real‑world, safe sanitization behavior and eliminate the multi‑character replacement issue entirely.
Concretely, in src/components/protocols/__tests__/protocol-details-sheet.test.tsx, locate the jest.mock('@/lib/utils', ...) block around line 17. Replace the current stripHtmlTags mock implementation:
stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''),with an implementation that calls sanitize-html with a strict configuration that removes all tags and attributes, returning plain text. We will first require/import sanitize-html at the top of the file, then define stripHtmlTags to call sanitizeHtml(html, { allowedTags: [], allowedAttributes: {} }) when html is non‑empty, and return an empty string otherwise. This preserves the existing functional contract (string in, string out, empty string for falsy input) while eliminating the problematic regex and making the sanitization robust to multi‑character issues.
Because we are only allowed to modify this file, we will add a new import (or require) for sanitize-html at the top next to the existing imports and adjust only the stripHtmlTags line within the jest.mock call.
| @@ -4,6 +4,7 @@ | ||
| import { CallProtocolsResultData } from '@/models/v4/callProtocols/callProtocolsResultData'; | ||
|
|
||
| import { ProtocolDetailsSheet } from '../protocol-details-sheet'; | ||
| import sanitizeHtml from 'sanitize-html'; | ||
|
|
||
| // Mock analytics | ||
| const mockTrackEvent = jest.fn(); | ||
| @@ -17,7 +18,9 @@ | ||
| jest.mock('@/lib/utils', () => ({ | ||
| formatDateForDisplay: jest.fn((date) => date ? '2023-01-01 12:00 UTC' : ''), | ||
| parseDateISOString: jest.fn((dateString) => dateString ? new Date(dateString) : null), | ||
| stripHtmlTags: jest.fn((html) => html ? html.replace(/<[^>]*>/g, '') : ''), | ||
| stripHtmlTags: jest.fn((html) => | ||
| html ? sanitizeHtml(html, { allowedTags: [], allowedAttributes: {} }) : '', | ||
| ), | ||
| })); | ||
|
|
||
| jest.mock('nativewind', () => ({ |
| @@ -174,7 +174,8 @@ | ||
| "react-query-kit": "~3.3.0", | ||
| "tailwind-variants": "~0.2.1", | ||
| "zod": "~3.23.8", | ||
| "zustand": "~4.5.5" | ||
| "zustand": "~4.5.5", | ||
| "sanitize-html": "^2.17.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@babel/core": "~7.26.0", |
| Package | Version | Security advisories |
| sanitize-html (npm) | 2.17.0 | None |
| if (!html) return ''; | ||
|
|
||
| // Remove HTML tags using regex | ||
| const withoutTags = html.replace(/<[^>]*>/g, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, the safest fix is to avoid hand-written regex-based HTML stripping and instead use a robust, battle-tested HTML sanitizer/stripping library. This reduces the risk of multi-character sanitization issues and other parsing edge cases that regexes cannot handle correctly. For this codebase, we should replace the current stripHtmlTags implementation with one that delegates HTML parsing/stripping to a library that knows how to handle broken tags, nested elements, and script/style content, and returns plain text.
The best targeted fix without changing external behavior too much is:
- Use a small, focused library to convert HTML to plain text (stripping all tags) and then perform the entity replacements/normalizations and trimming as before.
string-strip-htmlis a well-known npm package that safely removes HTML tags and is designed for exactly this purpose.
Concretely, insrc/lib/utils.ts:
- Add an import for
stripHtmlfromstring-strip-html. - Rewrite
stripHtmlTagsso that:- It first calls
stripHtml(html).resultto obtain HTML-stripped text. - Then runs the existing
.replacechain and.trim()on that plain text (preserving current normalization behavior).
- It first calls
- Keep the function signature and name unchanged so existing callers keep working.
No other regions of the file need changes.
| @@ -3,6 +3,7 @@ | ||
| import type { StoreApi, UseBoundStore } from 'zustand'; | ||
|
|
||
| import { Env } from './env'; | ||
| import { stripHtml } from 'string-strip-html'; | ||
|
|
||
| /** | ||
| * Call State Constants | ||
| @@ -166,8 +167,8 @@ | ||
| export function stripHtmlTags(html: string): string { | ||
| if (!html) return ''; | ||
|
|
||
| // Remove HTML tags using regex | ||
| const withoutTags = html.replace(/<[^>]*>/g, ''); | ||
| // Remove HTML tags using a robust HTML stripping library | ||
| const withoutTags = stripHtml(html).result; | ||
|
|
||
| // Replace common HTML entities | ||
| return withoutTags |
| if (!html) return ''; | ||
|
|
||
| // Remove HTML tags using regex | ||
| const withoutTags = html.replace(/<[^>]*>/g, ''); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
| return withoutTags | ||
| .replace(/ /g, ' ') | ||
| .replace(/&/g, '&') |
Check failure
Code scanning / CodeQL
Double escaping or unescaping High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to avoid double-unescaping when manually decoding HTML entities, the escape character & must be decoded last. This ensures that any & appearing as part of an entity sequence (like <, ", etc.) is handled before & turns &lt; into <, which would otherwise then be decoded again.
For this specific function, the minimal change that preserves existing behavior is to move the .replace(/&/g, '&') call to the end of the replacement chain, after all other entity replacements. This keeps all current mappings ( , <, >, ", ', ’, ‘, “, ”, —, –) but guarantees that & is only introduced after all other entity patterns have been processed, preventing subsequent unintended decodes. No new imports or helper methods are needed; only the order of the chained .replace() calls within stripHtmlTags in src/lib/utils.ts (lines 173–186) must be adjusted.
| @@ -172,7 +172,6 @@ | ||
| // Replace common HTML entities | ||
| return withoutTags | ||
| .replace(/ /g, ' ') | ||
| .replace(/&/g, '&') | ||
| .replace(/</g, '<') | ||
| .replace(/>/g, '>') | ||
| .replace(/"/g, '"') | ||
| @@ -183,6 +182,7 @@ | ||
| .replace(/”/g, '"') | ||
| .replace(/—/g, '—') | ||
| .replace(/–/g, '–') | ||
| .replace(/&/g, '&') | ||
| .trim(); | ||
| } | ||
|
|
No description provided.