Conversation
…s hex representation)
…ules, plus a few fixes to genuinely ambiguous tests
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| current = current[parts[i]]; | ||
| } | ||
| current[parts[parts.length - 1]] = value; |
Check warning
Code scanning / CodeQL
Prototype-polluting function Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
The safest minimal fix is to harden setNestedField by rejecting any path segment that can participate in prototype-chain mutation: __proto__, constructor, and prototype (not just __proto__). This preserves existing functionality for legitimate field paths while blocking known prototype pollution vectors.
In packages/db-connect/lib/shared.js, update setNestedField:
- Replace the current guard:
if (parts.includes('__proto__')) { return; }
- With a stronger segment validation that rejects all dangerous keys:
if (parts.some(part => part === '__proto__' || part === 'constructor' || part === 'prototype')) { return; }
No new imports or external dependencies are required.
Suggested changeset
1
packages/db-connect/lib/shared.js
| @@ -103,7 +103,7 @@ | ||
|
|
||
| function setNestedField(obj, path, value) { | ||
| const parts = path.split('.'); | ||
| if (parts.includes('__proto__')) { | ||
| if (parts.some(part => part === '__proto__' || part === 'constructor' || part === 'prototype')) { | ||
| return; | ||
| } | ||
| let current = obj; |
Copilot is powered by AI and may make mistakes. Always verify output.
* "npm test" tests all three adapters
boutell
commented
Apr 12, 2026
| .*.sw* | ||
|
|
||
| # claude-tools log files | ||
| claude-tools/**/*.log |
Member
Author
There was a problem hiding this comment.
claude-tools is meant to persist, but its logs are not
boutell
commented
Apr 12, 2026
| 'test/apos-build', | ||
| 'coverage' | ||
| 'coverage', | ||
| 'claude-tools' |
Member
Author
There was a problem hiding this comment.
claude code riffing scripts for its own test purposes, eslint not necessary
boutell
commented
Apr 12, 2026
| "main": "index.js", | ||
| "scripts": { | ||
| "pretest": "npm run lint", | ||
| "test": "npm run test:base && npm run test:missing && npm run test:assets && npm run test:esm", |
Member
Author
There was a problem hiding this comment.
asset tests are now a good citizen, don't need to be broken out separately anymore
* optimize $match when it is the first step in aggregation, don't fetch the whole collection 😜 * multipostgres listDatabases() and .db() should return and expect "fully qualified virtual database names," e.g. physical_db_name-schemaname
documentation improvements
…w on unrecognized operators. It should, and it should support the same mongodb operators that the regular find()
path does in postgres/sqlite (our official subset), unless there is an extraordinary reason not to.
* Similarly, the main query implementation for normal queries should throw on unrecognized operators if it doesn't already.
* The dump/restore programmatic APIs in db-connect concern me. These involve returning the entire database as a string, which could exhaust memory. This impacts both utilities and
also copyDatabase(). Could these APIs return and expect async iterators instead of strings?
* The test "anchored regex on an indexed field uses a btree index search" runs explain on a query that's hardcoded in the test. Instead these SQL based adapters should expose a means
to get the SQL for a query, so it can be directly tested. Otherwise this test proves nothing as changes to the adapter accumulate in future.
* Why is this test searching for "at least 1" and not exactly 1?
it('should find documents with null value', async function() {
const docs = await db.collection('test').find({ value: null }).toArray();
// MongoDB matches both null and missing fields with { value: null }
expect(docs.length).to.be.at.least(1);
});
* What is the maximum size of a db-connect document in the postgres and sqlite adapters?
* Update the copyright year in db-connect/LICENSE.md to 2025.
* The db-connect README mentions: sqlite://:memory: What happens if you try to use .db('some-name') with that? I think it would be best to just not support throwaway in-memory sqlite
databases because I doubt anyone would intentionally store a website in one.
myovchev
reviewed
Apr 20, 2026
| "build": "pnpm --recursive run build", | ||
| "lint": "pnpm --recursive run lint", | ||
| "test": "pnpm --recursive run test", | ||
| "test": "APOS_TEST_DB_PROTOCOL=postgres npm run test:main && APOS_TEST_DB_PROTOCOL=mongodb npm run test:main && APOS_TEST_DB_PROTOCOL=sqlite npm run test:main && APOS_TEST_DB_PROTOCOL=multipostgres npm run test:main", |
Contributor
There was a problem hiding this comment.
This should be probably pnpm instead npm.
myovchev
approved these changes
Apr 21, 2026
BoDonkey
approved these changes
Apr 22, 2026
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
4 participants
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.
No description provided.