chore: backport fixes from the v1x branch#10838
Merged
Merged
Conversation
jasonsaayman
added
priority::high
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 28 files
Confidence score: 3/5
- There is some merge risk because
lib/core/AxiosError.jshas a concrete behavior issue (6/10, high confidence): using{}for redaction lookups can falsely matchObject.prototypekeys likeconstructor/toString, leading to incorrect config redaction. - The other findings are low impact and non-functional (
lib/helpers/AxiosURLSearchParams.jstrailing commas and malformed comment text inREADME.md), so they are unlikely to cause runtime regressions. - Given one medium-severity, user-visible correctness issue in core error handling, this is not a no-risk merge even though most items are minor housekeeping.
- Pay close attention to
lib/core/AxiosError.js,lib/helpers/AxiosURLSearchParams.js,README.md- redaction key handling is the main risk; style/docs fixes are straightforward.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/helpers/AxiosURLSearchParams.js">
<violation number="1" location="lib/helpers/AxiosURLSearchParams.js:14">
P3: Remove newly added trailing commas in this `lib/` file to match repository lint/style rules (`no trailing commas`).</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:407">
P3: Fix the malformed comment text by removing the leading comma.</violation>
</file>
<file name="lib/core/AxiosError.js">
<violation number="1" location="lib/core/AxiosError.js:19">
P2: Use `Object.create(null)` instead of `{}` to avoid false-positive redaction from `Object.prototype` properties. A config key named `"constructor"` or `"toString"` would be incorrectly redacted because `{}["constructor"]` is truthy. The same fix was already applied to `descriptors` in this PR.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Member
Author
|
@cubic-dev-ai please review this one in depth! |
Contributor
@jasonsaayman I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
1 issue found across 29 files
Confidence score: 5/5
- This looks low risk to merge: the only reported issue is low severity (3/10) and is confined to documentation guidance rather than product/runtime code.
- The main concern is in
AGENTS.md, where references to non-existent workflow paths/command docs could mislead contributors and reduce trust in the agent instructions. - Pay close attention to
AGENTS.md- remove or correct fabricated-looking workflow/command references so contributor guidance is accurate.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:41">
P3: Custom agent: **Flag AI Slop and Fabricated Changes**
References non-existent workflow paths and command docs, making this guidance look fabricated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Backports security and hardening fixes from
v1.xtov0.x. Tightens header/config merging, default error redaction, cookie/XSRF handling, proxy bypass rules, and Unix socket usage, plus test stability on Node 12–24.Description
A clear and concise description of the PR.
Use this section for review hints, explanations or discussion points.
AxiosError.toJSON()now redacts sensitive keys by default; configurable viaconfig.redact; defaults exposed ondefaults.redact.withXSRFToken.NO_PROXYmatching handles canonical IPv4‑mapped IPv6 forms (e.g.,::ffff:127.0.0.1and::ffff:7f00:1).httpadapter: stripProxy-Authorizationwhen not using a proxy; gatesocketPathbehindallowedSocketPaths(string/array, normalized); misc hardening.xhradapter: stricter own‑property checks for config/headers.AxiosURLSearchParamskeeps%00encoded; encoding made consistent.formDataHeaderPolicy,redact,allowedSocketPathsto types; updateREADME.md/CHANGELOG.md; addAGENTS.md; refresh issue/PR templates;.gitignoretweak.dnsfor Node 12); OpenSSL‑legacy paths handled via existing test harness.Docs
Update
/docs/for:config.redactdefaults and override behavior.formDataHeaderPolicyand serializer depth caps.allowedSocketPathsgating and path normalization.hasOwnPropertyusage.%00behavior in params.NO_PROXYhandling of IPv4‑mapped IPv6.Testing
httpadapter safety (Proxy-Authorizationstripping,allowedSocketPaths), URL param%00, form‑data depth limits, and TS types.dns) and legacy OpenSSL flows.Semantic version impact
Breaking change.
Object.prototype.hasOwnProperty.call(obj, key)and avoid implicit string coercion of these objects.Written for commit 7fa296c. Summary will update on new commits.