Skip to content

fix: modernize website dependency security#2574

Merged
ranadeepsingh merged 2 commits into
microsoft:rana/fix-ado-docker-authfrom
ranadeepsingh:fix/website-security-backlog
Jul 26, 2026
Merged

fix: modernize website dependency security#2574
ranadeepsingh merged 2 commits into
microsoft:rana/fix-ado-docker-authfrom
ranadeepsingh:fix/website-security-backlog

Conversation

@ranadeepsingh

@ranadeepsingh ranadeepsingh commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Related Issues/PRs

What changes are proposed in this pull request?

Issue

The legacy website used Yarn 1, Node 18, Docusaurus 2.4.1, and React 16. Its
yarn.lock produced 112 Component Governance alerts, including two Critical
findings. The lock also depended on an obsolete dependency graph that could not
be remediated safely with isolated transitive upgrades.

Fix

  • Move the website to Node 20, npm, Docusaurus 3.10.2, and React 18.3.1.
  • Replace yarn.lock with an npm v3 lock containing 1,298 SHA-512 integrity
    records and no internal registry or mutable resolved URLs.
  • Apply targeted overrides for affected transitive packages and run npm ci,
    tests, and the production build in website CI.
  • Add a tested MDX 3 compatibility preprocessor that preserves fenced code
    blocks, plus the required Prism renderer and historical-link updates.
  • Update version-bump automation and contributor documentation from Yarn to
    local npm commands.
  • Pin Codecov CLI v11.3.1, verify its official SHA-256 before execution, and
    keep the upload token environment-only.
  • Narrow unit-test Key Vault access from the full vault to only
    codecov-token, load it immediately before the PR-only upload, and retain
    coverage uploads after failed tests with succeededOrFailed().

Security outcome

  • All prior Critical and High affected versions are removed.
  • 111 of the 112 original Component Governance alerts are eliminated. The ADO
    Component Governance task now reports one Low string_decoder@1.3.0 alert
    inherited through the current webpack-dev-server -> spdy development-only
    path.
  • The final npm audit has no Critical, Moderate, or Low findings. Its only
    vulnerable package is brace-expansion@1.1.16
    (GHSA-mh99-v99m-4gvg),
    which npm reports as fixAvailable: false. It is inherited from the latest
    Docusaurus/serve-handler static-server path and receives only trusted
    configured glob patterns; this PR avoids introducing an unmaintained fork or
    install-time mutation to hide the advisory.

How is this patch tested?

  • I have written tests and confirmed the proposed change works.
  • npm test under Node 20: 2 tests passed.
  • npm run build under Node 20: Docusaurus production build passed.
  • npm ls --all: dependency tree valid with fast-uri@3.1.4.
  • python -m pytest -q scripts/test_bump_version.py: 177 tests passed.
  • black --check --extend-exclude 'docs/' scripts/bump-version.py scripts/test_bump_version.py
  • sbt scalastyle test:scalastyle
  • Changed YAML and JSON syntax validation.
  • Lock consistency, integrity algorithm, registry URL, old-alert comparison,
    and npm audit checks.
  • ADO Component Governance detection succeeded and reported one Low alert,
    down from the original 112 alerts including two Critical findings.
  • GitHub Website Deploy build passed from a fresh public-registry install.
  • Full ADO build 228133377
    passed every website, style, build, publish, Python, R, CPU/GPU Databricks,
    Fabric, and all but one unit job. The sole failure was one agent receiving an
    azureedge.net certificate for msdata.visualstudio.com while obtaining its
    Key Vault OIDC token; the same Key Vault task passed in 61 sibling jobs.
  • Targeted ADO unit rerun 228141069
    passed, confirming the failure was transient infrastructure rather than a
    code regression.
  • Latest hardening commit 9a5e7d6f9f passed GitHub Website Deploy, CodeQL,
    dependency review, YAML parsing, Black (178 files), Scala style, and
    whitespace validation.
  • Clean targeted ADO build 228246262
    passed BuildDocker, conda setup, and all 40 unit-test matrix jobs on the
    latest commit. The PR-only Codecov secret task remains intentionally
    excluded from manual builds because Build.Reason is system-controlled.

Does this PR change any dependencies?

  • No. You can skip this section.
  • Yes. The website moves from Yarn to npm and upgrades to Node 20,
    Docusaurus 3.10.2, React 18.3.1, current math/Prism integrations, and patched
    transitive dependency versions recorded in website/package-lock.json.

Does this PR add a new feature? If so, have you added samples on website?

  • No. This is dependency, compatibility, and supply-chain maintenance.
  • Yes. Make sure you have added samples following the template steps.

AB#5469322

## Summary
Replace the legacy Yarn and Docusaurus 2 website toolchain with Node 20,
npm, Docusaurus 3, and React 18. Add targeted dependency overrides, a
portable SHA-512 package lock, MDX 3 compatibility handling and tests, and
checksum verification for the pinned Codecov CLI.

## Prompting Intent
The engineer asked for a deeply reviewed follow-up to the ADO pipeline repair
that resolves the website/yarn.lock security backlog without reducing test
coverage, introducing regressions, leaking credentials, or weakening
supply-chain protections.

## Linked Sources
- ADO Bug 5469322: https://dev.azure.com/msdata/A365/_workitems/edit/5469322
- Parent pipeline repair PR: microsoft#2573
- Component Governance build: https://msdata.visualstudio.com/A365/_build/results?buildId=228104679&view=results
- Residual upstream advisory: GHSA-mh99-v99m-4gvg

## Rationale
The legacy dependency graph could not be remediated safely with isolated
transitive bumps, so the website was moved to the current supported Docusaurus
and React generations. npm provides a deterministic lock and CI-native cache;
the committed lock omits environment-specific registry URLs and verifies every
registry artifact with SHA-512. A tested preprocessor preserves historical
documentation while adapting only legacy MDX constructs outside fenced code.
Codecov is pinned and checksum-verified rather than downloaded from a mutable
latest URL. The remaining brace-expansion advisory has no compatible upstream
fix and is limited to trusted static-server glob configuration, so no risky
vendored fork or node_modules mutation was introduced.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

Copy link
Copy Markdown

Hey @ranadeepsingh 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

  • fix: Fix LightGBM crashes with empty partitions
  • feat: Make HTTP on Spark back-offs configurable
  • docs: Update Spark Serving usage
  • build: Add codecov support
  • perf: improve LightGBM memory usage
  • refactor: make python code generation rely on classes
  • style: Remove nulls from CNTKModel
  • test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

ADO validation note: /azp run was filtered because this is a stacked PR whose base branch is outside the pipeline PR trigger list. The exact PR commit was therefore queued manually. Full build 228133377 passed every website, style, publish, Python, R, Databricks, Fabric, and all but one unit job; one agent hit a transient Key Vault OIDC TLS certificate mismatch while the same task passed in 61 sibling jobs. Targeted unit rerun 228141069 passed all unit jobs, and the PR status is green.

AB#5469322

## Summary
Load only the Codecov token immediately before coverage upload instead of
loading the complete build Key Vault before unit tests. Restore the readability
spacing that was unintentionally removed from the legacy Helm Dockerfiles.

## Prompting Intent
The engineer asked for a file-by-file review of the stacked pipeline changes,
with particular attention to secret leakage, unnecessary changes, the conda
cache condition, publishing log levels, and preservation of Dockerfile
readability.

## Linked Sources
- Website security bug: https://dev.azure.com/msdata/A365/_workitems/edit/5469322
- Pipeline repair bug: https://dev.azure.com/msdata/A365/_workitems/edit/5467636
- Stacked pull request: microsoft#2574
- Parent pipeline repair: microsoft#2573
- Full ADO validation: https://msdata.visualstudio.com/A365/_build/results?buildId=228133377&view=results

## Rationale
The earlier Key Vault placement ensured Codecov could upload after a failed
test, but it also made every vault secret available before arbitrary unit-test
execution. A late AzureKeyVault task with succeededOrFailed preserves failure
uploads while filtering access to codecov-token only. The Helm blank-line
removals had no functional or warning-related benefit, so restoring them keeps
the original visual grouping without changing image behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranadeepsingh
ranadeepsingh merged commit 321ed02 into microsoft:rana/fix-ado-docker-auth Jul 26, 2026
50 checks passed
@ranadeepsingh
ranadeepsingh deleted the fix/website-security-backlog branch July 26, 2026 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant