Skip to content

Conversation

@jgongd
Copy link
Contributor

@jgongd jgongd commented Mar 10, 2025

Description

Upgrade to fix a vulnerability in the Try-it-out feature.
GHSA-qrmm-w75w-3wpx

We've chosen the SwaggerUI standalone installation that doesn't require npm: https://github.com/swagger-api/swagger-ui/blob/80d56c9518af7ad523f7171815c9da836551e259/docs/usage/installation.md#plain-old-htmlcssjs-standalone

Test Plan

  1. make -C proto build generates the latest api.swagger.json
  2. make -C docs build
  3. make -C docs live

A determined rest api page is expected:
image

Checklist

  • Changes have been manually QA'd
  • New features have been approved by the corresponding PM
  • User-facing API changes have the "User-facing API Change" label
  • Release notes have been added as a separate file under docs/release-notes/
    See Release Note for details.
  • Licenses have been included for new code which was copied and/or modified from any external code

Upgrade to fix a vulnerability in the Try-it-out feature.
GHSA-qrmm-w75w-3wpx
@jgongd jgongd requested review from hkang1 and mackrorysd March 10, 2025 15:18
@cla-bot cla-bot bot added the cla-signed label Mar 10, 2025
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Mar 10, 2025
@determined-ci determined-ci requested a review from a team March 10, 2025 15:18
@netlify
Copy link

netlify bot commented Mar 10, 2025

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 19866da
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/67cf05ca315aa10008cb772f

Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question about the replace() function, the rest look good


if (/code|token|error/.test(window.location.hash)) {
qp = window.location.hash.substring(1);
qp = window.location.hash.substring(1).replace('?', '&');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: what is the replace for? do question marks get entered into the hash somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears to fix a parsing error when someone uses casdoor: swagger-api/swagger-ui#8149

@jgongd jgongd requested a review from hkang1 March 11, 2025 15:28
Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the upgrade and fix

@jgongd jgongd merged commit b3b310a into main Mar 11, 2025
79 of 91 checks passed
@jgongd jgongd deleted the jgong/patch-swaggerui branch March 11, 2025 16:51
jgongd added a commit that referenced this pull request Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants