Repository navigation
Conversation
Co-authored-by: miqui <5511018+miqui@users.noreply.github.com>
lornajane
left a comment
There was a problem hiding this comment.
I am surprised to see this pull request given our AI policy. What is it's origin and how can we ask follow up questions about the purpose of the change?
I received an email notification from GH regarding these sensitive issues. There should be no surprise here. It's good that GH is doing this for the project on our behalf. I then took it upon myself to investigate (in my judgment), fired up my GH Copilot against the repo to fix these, and reviewed everything. I always do. Btw, the learn repo has similar problems. If you have any questions not already answered in this PR, let me know. |
|
Grokking back on the AI policy: The policy permits AI-assisted contributions with disclosure and human accountability(which I believe I have done), while prohibiting AI-generated issue comments, discussion replies, and PR descriptions. Nothing in it specifically bans contributing dependency security fixes. Perhaps here I could have done a better job in the original text/comments added by my agent (following my instructions). |
|
@miqui I don't understand why we wouldn't fix this upstream in build-infra? |
|
@miqui I had GPT look at this PR and whether the fix belongs here or in build-infra, and it belongs in build-infra with some caveats. GPT-written explanation (provided due to our general position that using AI is the only way our build infra work is going to get done): I think this should primarily be fixed in OpenAPI-Specification has only one direct JavaScript dependency:
Adding these resolutions here would make OpenAPI-Specification responsible for build-infra's transitive dependencies. We would also need equivalent overrides in every other specification repository using build-infra, potentially giving each consumer a different dependency graph. Avoiding that duplicated dependency maintenance is one of the reasons build-infra was introduced. There is one complication: copying this The preferable process is:
The exact dependency pins need additional consideration. Current This PR is also based on the older build-infra commit My recommendation is to supersede this PR with a build-infra change, followed by normal downstream dependency refreshes. If temporary consumer resolutions are unavoidable, they should be coordinated across all build-infra consumers rather than introduced only here. |
|
TDC: should be fixed: primarily be fixed in OAI/build-infra. Closing this PR. @miqui todo: will focus on build-infra instead. |
Address the six reported dependency alerts with targeted updates and a Yarn-regenerated lockfile. Preserve the build-infra commit, markdown-it 15.0.1, and separate undici major versions.
Dependency updates
Resolution scope
Override only markdownlint-cli2’s exact smol-toml, js-yaml, and markdown-it pins. Other updates satisfy existing ranges. Verified advisories require later fixes than the suggested joi, smol-toml, and brace-expansion releases; smol-toml 1.9.x retains the parser API used by markdownlint.
Validation
Node 24 / Yarn 4.18.0: immutable installation, all eight tests, and parser compatibility checks passed. All eight locked entries and seven installed copies are outside verified affected ranges. Markdown lint passed; external link validation failed on DNS resolution.
Remaining findings
Full audit still reports unchanged vulnerabilities in braces, KaTeX, and source-map-js, plus a whatwg-encoding deprecation. None of the six requested packages remains flagged. Read-only review found no significant issues; CodeQL had no analyzable source changes.