-
Notifications
You must be signed in to change notification settings - Fork 5
Sync dev with master #71
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
Conversation
| // 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
<script
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
General approach: Avoid a single-pass multi-character regex that can leave partially stripped unsafe sequences. Either use a robust HTML/text extraction library or apply the regex replacement repeatedly until no tags remain. Since we are in a small Jest mock and should avoid adding heavy dependencies if possible, a looped replacement is an appropriate fix that preserves existing behavior (removing tags, leaving text).
Concrete best fix here:
- Keep the idea of stripping tags, but ensure that all occurrences of
<...>patterns are removed, even if new ones are formed after a replacement. - Replace the single call
htmlContent.replace(/<[^>]*>/g, '')with a small loop that keeps replacing until the string no longer changes. - This does not change public behavior of the tests (still removes tags and trims whitespace), but eliminates the incomplete sanitization pattern CodeQL is warning about.
- No new imports are required; we can implement the loop inline in the mock where
textContentis defined.
Specific change:
-
In
src/components/contacts/__tests__/contact-notes-list.test.tsx, around line 27, replace:const textContent = htmlContent.replace(/<[^>]*>/g, '').trim();
with:
let textContent = htmlContent; let previous: string; do { previous = textContent; textContent = textContent.replace(/<[^>]*>/g, ''); } while (textContent !== previous); textContent = textContent.trim();
This keeps the functionality (strip tags, then trim) but ensures all tags are removed even in pathological inputs.
-
Copy modified lines R26-R34
| @@ -23,8 +23,15 @@ | ||
| 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(); | ||
| // Simple extraction of text content from HTML (removing tags). | ||
| // Apply the replacement repeatedly to ensure all tags are removed. | ||
| let textContent = htmlContent; | ||
| let previous: string; | ||
| do { | ||
| previous = textContent; | ||
| textContent = textContent.replace(/<[^>]*>/g, ''); | ||
| } while (textContent !== previous); | ||
| textContent = textContent.trim(); | ||
|
|
||
| return React.createElement(View, { | ||
| ...props, |
| 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
<script
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, the fix is to avoid incomplete, home‑rolled regex stripping of HTML and instead rely on a well‑tested sanitization library or, at minimum, a more robust pattern that does not let <script or similar constructs survive. Since we are constrained to only edit this test file, the best approach is to change the mock implementation of stripHtmlTags so it uses a safer sanitization strategy.
Concretely, we will update the jest.mock('@/lib/utils', ...) block so that stripHtmlTags no longer uses the vulnerable html.replace(/<[^>]*>/g, ''). A minimal, safer approach within this context is to first remove any <script...>...</script> blocks with a script‑focused regex, then strip remaining tags. That addresses CodeQL’s specific concern about residual <script content while preserving the overall behavior (removing HTML tags for display in tests). We keep stripHtmlTags as a Jest mock (jest.fn) so existing tests that assert on mocking behavior continue to work.
Changes needed:
- In
src/components/protocols/__tests__/protocol-card.test.tsx, replace the implementation ofstripHtmlTagsinside thejest.mockwith a version that:- Returns
''for falsy input (same as before). - Removes
<script>blocks using a reasonably standard script‑tag regex. - Removes remaining tags with a tag‑stripping regex.
No new imports are required, as we only use built‑in string/regex operations inside the mock.
- Returns
-
Copy modified lines R12-R19
| @@ -9,7 +9,14 @@ | ||
| 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) => { | ||
| if (!html) { | ||
| return ''; | ||
| } | ||
| // Remove script tags and their content first, then strip remaining HTML tags. | ||
| const withoutScripts = html.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, ''); | ||
| return withoutScripts.replace(/<[^>]*>/g, ''); | ||
| }), | ||
| })); | ||
|
|
||
| describe('ProtocolCard', () => { |
| 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
<script
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, to fix incomplete multi-character sanitization when stripping HTML, avoid ad‑hoc multi-character regexes like /\<[^>]*>/g and use a well-tested sanitization approach. For tests that just need to remove tags, a safer, character-based or looped replacement, or a battle-tested library such as sanitize-html, should be used instead of the fragile regex.
For this specific file, the simplest robust change is to replace the regex-based stripHtmlTags mock with an implementation that repeatedly removes any <...> tag-like construct until none remain. This directly addresses the “unsafe text reappearing” issue by looping until a fixpoint rather than relying on a single global replacement. Functionality from the test’s perspective stays the same—tags are stripped and plain text is returned—but the sanitization no longer suffers from the multi-character replacement weakness.
Concretely:
- In
src/components/protocols/__tests__/protocol-details-sheet.test.tsx, in the Jest mock of'@/lib/utils', change thestripHtmlTagsimplementation. - Replace
html.replace(/<[^>]*>/g, '')with a small loop that keeps applying the same regex until the string stops changing. This avoids adding new dependencies and stays within the provided snippet. - No new imports are required; the logic uses only built-in string and regex functionality.
-
Copy modified lines R20-R33
| @@ -17,7 +17,20 @@ | ||
| 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) => { | ||
| if (!html) { | ||
| return ''; | ||
| } | ||
|
|
||
| let previous; | ||
| let sanitized = html; | ||
| do { | ||
| previous = sanitized; | ||
| sanitized = sanitized.replace(/<[^>]*>/g, ''); | ||
| } while (sanitized !== previous); | ||
|
|
||
| return sanitized; | ||
| }), | ||
| })); | ||
|
|
||
| jest.mock('nativewind', () => ({ |
| 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 2 days ago
To fix the issue, explicitly define permissions: for the workflow so that the GITHUB_TOKEN is restricted to the least privileges required. Since the shown jobs only need to read repository contents (checkout, install, test) and do not modify the repo (no pushes, no PR updates, no releases using GITHUB_TOKEN are evident), the minimal safe configuration is contents: read at the workflow root. Individual jobs can later override this if needed, but that’s outside the current change scope.
The single best change with minimal functional impact is:
- Add a top-level
permissions:block right after thename:and beforeon:in.github/workflows/react-native-cicd.yml. - Set
contents: readthere so all jobs (check-skip,test,build-and-deploy, etc.) inherit read-only repo access by default.
No imports or additional methods are needed—this is purely a YAML workflow configuration change in .github/workflows/react-native-cicd.yml.
-
Copy modified lines R3-R5
| @@ -1,5 +1,8 @@ | ||
| name: React Native CI/CD | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] |
| 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 2 days ago
To fix this, we should explicitly define minimal GITHUB_TOKEN permissions in the workflow. Since no job appears to require write access to repository contents, issues, or pull requests, we can set contents: read (and nothing else) at the workflow root. Defining permissions at the root applies to all jobs (check-skip, test, and build-and-deploy) unless they override it, satisfying CodeQL while following least privilege.
The best fix is to add a top-level permissions block right after the name: or on: section in .github/workflows/react-native-cicd.yml. This keeps existing behavior for the CI/CD pipeline (it only needs to read the repo) while constraining the GITHUB_TOKEN. No imports or additional methods are needed because this is a YAML workflow configuration change only. If a future job needs elevated permissions (for example, to push tags or create a GitHub Release), that specific job should define its own permissions block overriding the workflow default.
Concretely:
- In
.github/workflows/react-native-cicd.yml, after thename: React Native CI/CDline (line 1), insert:
permissions:
contents: readThis will address the CodeQL finding for the test job (and all other jobs) by explicitly limiting GITHUB_TOKEN to read-only repository contents.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: React Native CI/CD | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| 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 2 days ago
To fix the issue, explicitly declare GITHUB_TOKEN permissions either at the workflow root or per job, granting only what each job needs. The check-skip and test jobs only require read access to repository contents to check out code, so they can use contents: read. The build-and-deploy job uploads artifacts and creates GitHub Releases via ncipollo/release-action@v1, which requires write access to repository contents/releases; the minimal appropriate scope is contents: write. Declaring these permissions removes reliance on the repository’s default token permissions and follows least-privilege principles.
Concretely, in .github/workflows/react-native-cicd.yml:
- Add a workflow-level
permissions: contents: readnear the top (aftername:or afteron:), so all jobs default to read-only. - Override this in the
build-and-deployjob by adding apermissions:block under that job withcontents: write. - We do not need additional scopes (
pull-requests,issues, etc.) because nothing in the shown snippet writes to those resources.
No imports or additional methods are needed because this is a YAML workflow configuration change only.
-
Copy modified lines R3-R5 -
Copy modified lines R120-R121
| @@ -1,5 +1,8 @@ | ||
| name: React Native CI/CD | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] | ||
| @@ -114,6 +117,8 @@ | ||
| build-and-deploy: | ||
| needs: test | ||
| if: (github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master')) || github.event_name == 'workflow_dispatch' | ||
| permissions: | ||
| contents: write | ||
| strategy: | ||
| matrix: | ||
| platform: [android, ios] |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the 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 |
| const decoded = he.decode(html, { isAttributeValue: false }); | ||
|
|
||
| // Remove HTML tags from the decoded string | ||
| const withoutTags = decoded.replace(/<[^>]*>/g, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
<script
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
General approach: avoid relying on a multi-character, tag-shaped regex that can be susceptible to overlapping/malformed input. For a function whose purpose is to return plain text with no HTML semantics, the safest strategy is to (1) decode entities, (2) use a robust library to strip all tags, or if that is not available, (3) explicitly remove tag-like patterns and then remove any remaining </> characters so that no executable HTML can remain. This ensures that even if a crafted input defeats the tag regex, the remaining angle brackets needed to form <script> are removed.
Best fix within constraints: we should keep the existing behavior (decode then strip tags and trim whitespace) but strengthen the stripping step to avoid incomplete multi-character sanitization. Since adding a full HTML-sanitizer dependency may be heavy and we’re limited to the shown snippet, we can mitigate the CodeQL finding by:
- Still decoding with
he.decode. - First removing any sequences that look like tags with
<[^>]*>(as today). - Then removing all remaining
<and>characters in the intermediate string; this eliminates any chance that<scriptor any other HTML tag-like sequence remains executable. - Returning the result trimmed.
This keeps the function’s intention (“plain text with all HTML tags removed”) and only slightly changes behavior: previously, literal </> that were not part of tags might survive; now they will be removed, which is typically acceptable for a tag-stripping helper and is safer from an injection standpoint.
Concretely, in src/lib/utils.ts, lines 173–176 should be updated so that after decoded.replace(/<[^>]*>/g, ''), we also strip any residual < or > characters via a second .replace(/[<>]/g, ''). No new imports are required, and the rest of the file remains unchanged.
-
Copy modified line R173 -
Copy modified lines R176-R179
| @@ -170,10 +170,13 @@ | ||
| // Decode all HTML entities first using 'he' library | ||
| const decoded = he.decode(html, { isAttributeValue: false }); | ||
|
|
||
| // Remove HTML tags from the decoded string | ||
| // First, remove HTML tag-like patterns from the decoded string | ||
| const withoutTags = decoded.replace(/<[^>]*>/g, ''); | ||
|
|
||
| return withoutTags.trim(); | ||
| // Then, remove any remaining angle brackets to avoid residual tag fragments | ||
| const fullySanitized = withoutTags.replace(/[<>]/g, ''); | ||
|
|
||
| return fullySanitized.trim(); | ||
| } | ||
|
|
||
| const monthNames = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']; |
|
Approve |
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 PR is approved.
No description provided.