Skip to content

Apply security and bug fixes from 87a677c81 to main.js #6069

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.

Sign up for GitHub

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

Jump to bottom
Open
jdalton wants to merge 2 commits into
base: amd
Choose a base branch
from
Open

Apply security and bug fixes from 87a677c81 to main.js #6069

jdalton wants to merge 2 commits into from
+168 −32

Conversation

@jdalton
Copy link
Member

@jdalton jdalton commented Dec 17, 2025

  • Fix prototype pollution in baseSet and baseUnset
  • Fix cyclic value comparison in equalArrays and equalObjects
  • Fix command injection vulnerability in template (variable validation, sourceURL whitespace)
  • Add array iteratee handling in baseOrderBy
  • Add early return for empty arrays in baseSortedIndexBy
  • Fix sortBy JSDoc example ages
  • Fix setCacheHas JSDoc return type

@jdalton
Apply security and bug fixes from 87a677c to main.js
56eaa4b 
- Fix prototype pollution in baseSet and baseUnset
- Fix cyclic value comparison in equalArrays and equalObjects
- Fix command injection vulnerability in template (variable validation, sourceURL whitespace)
- Add array iteratee handling in baseOrderBy
- Add early return for empty arrays in baseSortedIndexBy
- Fix sortBy JSDoc example ages
- Fix setCacheHas JSDoc return type
jonchurch
jonchurch reviewed Dec 17, 2025
View reviewed changes
Copy link
Member

@jonchurch jonchurch left a comment

Choose a reason for hiding this comment

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

reviewing...

@falsyvalues
Copy link
Member

falsyvalues commented Dec 17, 2025

@jdalton baseTrim and trimmedEndIndex is missing.

jonchurch
jonchurch requested changes Dec 17, 2025
View reviewed changes
Copy link
Member

@jonchurch jonchurch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@falsyvalues beat me by 1 minute!

This is missing the baseTrim and baseTrimEndIndex changes

We also havea JSDoc update that hasn't landed in main yet, not a blocker but we want to make sure we land that change into main.

falsyvalues
falsyvalues requested changes Dec 17, 2025
View reviewed changes
Copy link
Member

@falsyvalues falsyvalues left a comment

Choose a reason for hiding this comment

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

https://github.com/lodash/lodash/blob/main/lodash.js#L1017
https://github.com/lodash/lodash/blob/main/lodash.js#L1364

@jonchurch
Copy link
Member

jonchurch commented Dec 17, 2025 •
edited
Loading

Luckily the commands to regenerate this file are captured in the header comment.

It's hinky bc it requires using @bnjmnt4n's forked lodash-cli (as the lodas-cli is archived, but changes were needed to it, and we need those changes here as well)

  1. Checkout the branch you want to compare against
  2. Install the lodash-cli fork: npm i -D github:bnjmnt4n/lodash-cli
  3. Copy main's lodash.js as the source: git show main:dist/lodash.js > node_modules/lodash/lodash.js (make sure you've got latest main!)
  4. Run the build command from the header comment:
    • AMD: npx lodash exports=amd -d -o ./main.js
  5. Check the diff: git diff main.js

Caveats which are indicative of the issues w/ treating main's lodash.js as source of truth, but it currently having drifted slightly:

  • The regenerated file will have VERSION from main's source (4.17.21), so you'll need to bump it after (bumping version in main would prevent this, but we were saving it for last)
  • The setCacheHas JSDoc has a bug in main (@returns {number} should be @returns {boolean}) - this regresses the fix that was in the distribution branches
  • For minified files, postprocess.js in lodash-cli replaces the full license header with a short one, so those need manual handling to replace it with the longer one (remnant of the license being updated manually, but the cli not getting the update)

@jonchurch
regenerate main.js from main branch lodash.js
bcbbbbc 
plus the setCacheHas JSDoc tweak bc see it fix it aye
@jonchurch
Copy link
Member

jonchurch commented Dec 17, 2025 •
edited
Loading

I pushed the result of regenerating from main in bcbbbbc

@jdalton jdalton marked this pull request as ready for review December 18, 2025 01:19
@jdalton
Copy link
Member Author

jdalton commented Dec 18, 2025

@falsyvalues did @jonchurch's fix tackle it?

falsyvalues
falsyvalues approved these changes Dec 18, 2025
View reviewed changes
Copy link
Member

@falsyvalues falsyvalues left a comment

Choose a reason for hiding this comment

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

Except of this additional change, the rest looks good!

 * @returns {boolean} Returns `true` if `value` is found, else `false`.

@jonchurch jonchurch mentioned this pull request Dec 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonchurch jonchurch jonchurch requested changes

@falsyvalues falsyvalues falsyvalues approved these changes

@UlisesGascon UlisesGascon UlisesGascon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jdalton @falsyvalues @jonchurch @UlisesGascon