Skip to content

Commit 45bb99a

Browse files
authored
🥽 fix: Stop the DocumentDB Guard Flagging Mongoose's Document $where (#15686)
The DocumentDB compatibility guard flagged Mongoose per-document save-condition bag reads and writes (document.$where) in tenantIsolation.ts as the unsupported $where operator, leaving dev red on Tests: data-schemas. The guard now judges a dotted $where only where the syntax is unambiguous: a call in any form, or code assigned through any operator or wrapper, is an offense; a read or a non-literal assignment is not claimed, with filter.$where = predicate stated as the one declared limit and the method sweep and live cluster run as its backstop. Every other way of writing the operator remains an offense. unwrapExpression also peels angle-bracket assertions, closing a gap in pipeline-update detection.
1 parent ea3c61d commit 45bb99a

1 file changed

Lines changed: 210 additions & 5 deletions

File tree

‎packages/data-schemas/src/methods/documentdb.spec.ts‎

Lines changed: 210 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import ts from 'typescript';
2323
* that talks to MongoDB — this package, `packages/api`, and `api` — because the
2424
* regression class is repo-wide and new backend code lands in `packages/api`.
2525
* Dataflow is followed within a file only — a pipeline imported from another
26-
* module is out of reach — and the method sweep and the live cluster run are
27-
* the completeness backstops, not this guard.
26+
* module is out of reach — and a dotted `$where` is judged only where syntax is
27+
* unambiguous (see `isUnjudgedDottedWhere`); the method sweep and the live
28+
* cluster run are the completeness backstops, not this guard.
2829
* If a construct here becomes genuinely necessary, the fix is a compatible
2930
* rewrite, not an exception list: `misc/documentdb/audit.documentdb.spec.ts`
3031
* re-adjudicates any of this against a real cluster.
@@ -186,11 +187,13 @@ function parse(fileName: string, source: string): ts.SourceFile {
186187
return ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true);
187188
}
188189

189-
/** Peels casts and parentheses so `[...] as PipelineStage[]` is still an array. */
190+
/** Peels casts, assertions and parentheses so `[...] as PipelineStage[]` and
191+
* `<PipelineStage[]>[...]` are still arrays. */
190192
function unwrapExpression(expression: ts.Expression): ts.Expression {
191193
let current = expression;
192194
while (
193195
ts.isAsExpression(current) ||
196+
ts.isTypeAssertionExpression(current) ||
194197
ts.isSatisfiesExpression(current) ||
195198
ts.isParenthesizedExpression(current) ||
196199
ts.isNonNullExpression(current)
@@ -404,15 +407,114 @@ function findPipelineUpdates(sourceFile: ts.SourceFile): string[] {
404407
* ignoring prose — the rewrites explain themselves by naming the construct —
405408
* and type members, which never reach the engine (Mongoose documents declare
406409
* a `$where` field). */
410+
/** The operator's value is code; the document bag's value is field predicates. */
411+
function isJavaScriptSource(expression: ts.Expression): boolean {
412+
const unwrapped = unwrapExpression(expression);
413+
return (
414+
ts.isStringLiteralLike(unwrapped) ||
415+
ts.isTemplateExpression(unwrapped) ||
416+
ts.isArrowFunction(unwrapped) ||
417+
ts.isFunctionExpression(unwrapped)
418+
);
419+
}
420+
421+
/**
422+
* A dotted `$where` is Mongoose's per-document save-condition bag on a document
423+
* (`mongoose/lib/model.js` copies its keys into the save filter as ordinary field
424+
* predicates, so nothing named `$where` reaches the server) and the JavaScript
425+
* evaluation operator on a query or filter, and syntax alone cannot tell the two
426+
* apart. So it is judged only where the syntax is unambiguous: a CALL
427+
* (`query.$where(js)`, parenthesised, or through `call`/`apply`/`bind`) or an
428+
* assignment of CODE (a string, template, arrow or function). A read, or an
429+
* assignment of anything else — `document.$where = { … }` in
430+
* `tenantIsolation.ts`, but equally `filter.$where = predicate` — is not claimed
431+
* either way; for the latter the method sweep and the live cluster run are the
432+
* backstop. Every other way of writing the operator (`{ $where: … }`,
433+
* `'$where'`, `obj['$where']`) remains an offense.
434+
*/
435+
const CALL_FORWARDERS = new Set(['call', 'apply', 'bind']);
436+
const CODE_ASSIGNMENT_OPERATORS = new Set([
437+
ts.SyntaxKind.EqualsToken,
438+
ts.SyntaxKind.PlusEqualsToken,
439+
ts.SyntaxKind.QuestionQuestionEqualsToken,
440+
ts.SyntaxKind.BarBarEqualsToken,
441+
ts.SyntaxKind.AmpersandAmpersandEqualsToken,
442+
]);
443+
444+
/** Steps outward through the wrappers `unwrapExpression` peels inward, so a
445+
* call on or an assignment to `(x)`, `x as T`, `<T>x`, `x satisfies T` or `x!`
446+
* is still one on `x`. */
447+
function outermostWrapper(node: ts.Node): ts.Node {
448+
let current = node;
449+
while (
450+
ts.isParenthesizedExpression(current.parent) ||
451+
ts.isAsExpression(current.parent) ||
452+
ts.isTypeAssertionExpression(current.parent) ||
453+
ts.isSatisfiesExpression(current.parent) ||
454+
ts.isNonNullExpression(current.parent)
455+
) {
456+
current = current.parent;
457+
}
458+
return current;
459+
}
460+
461+
function isInvoked(callee: ts.Node): boolean {
462+
const use = outermostWrapper(callee).parent;
463+
return ts.isCallExpression(use) && use.expression === outermostWrapper(callee);
464+
}
465+
466+
/** The member name a node is accessed through, whether dotted (`x.call`) or by a
467+
* string element (`x['call']`). */
468+
function memberName(use: ts.Node, receiver: ts.Node): string | undefined {
469+
if (ts.isPropertyAccessExpression(use) && use.expression === receiver) {
470+
return use.name.text;
471+
}
472+
if (ts.isElementAccessExpression(use) && use.expression === receiver) {
473+
const argument = unwrapExpression(use.argumentExpression);
474+
return ts.isStringLiteralLike(argument) ? argument.text : undefined;
475+
}
476+
return undefined;
477+
}
478+
479+
/** A direct call, or a call through `call`/`apply`/`bind` — the forwarder itself
480+
* must be invoked, so a bag field that merely shares one of those names is not a call. */
481+
function isCalled(access: ts.PropertyAccessExpression): boolean {
482+
if (isInvoked(access)) {
483+
return true;
484+
}
485+
const target = outermostWrapper(access);
486+
const forwarder = memberName(target.parent, target);
487+
return forwarder != null && CALL_FORWARDERS.has(forwarder) && isInvoked(target.parent);
488+
}
489+
490+
function isUnjudgedDottedWhere(node: ts.Node): boolean {
491+
if (!ts.isIdentifier(node) || node.text !== '$where') {
492+
return false;
493+
}
494+
const access = node.parent;
495+
if (!ts.isPropertyAccessExpression(access) || access.name !== node || isCalled(access)) {
496+
return false;
497+
}
498+
const target = outermostWrapper(access);
499+
const use = target.parent;
500+
const assignsCode =
501+
ts.isBinaryExpression(use) &&
502+
use.left === target &&
503+
CODE_ASSIGNMENT_OPERATORS.has(use.operatorToken.kind) &&
504+
isJavaScriptSource(use.right);
505+
return !assignsCode;
506+
}
507+
407508
function findForbiddenTokens(sourceFile: ts.SourceFile): string[] {
408509
if (OPERATOR_GUARDS.has(sourceFile.fileName)) {
409510
return [];
410511
}
411512
const offenses: string[] = [];
412513
const visit = (node: ts.Node): void => {
413514
if (
414-
ts.isStringLiteralLike(node) ||
415-
(ts.isIdentifier(node) && !ts.isPropertySignature(node.parent))
515+
!isUnjudgedDottedWhere(node) &&
516+
(ts.isStringLiteralLike(node) ||
517+
(ts.isIdentifier(node) && !ts.isPropertySignature(node.parent)))
416518
) {
417519
for (const token of FORBIDDEN_TOKENS) {
418520
if (node.text === token || node.text.startsWith(`${token}.`)) {
@@ -676,6 +778,10 @@ describe('Amazon DocumentDB compatibility', () => {
676778
'annotated variable with a builder initializer',
677779
`const update: PipelineStage[] = importedBuilder();\nModel.updateMany(filter, update);`,
678780
],
781+
[
782+
'angle-bracket cast literal',
783+
`Model.updateOne(filter, <PipelineStage[]>[{ $set: { a: 1 } }]);`,
784+
],
679785
])('flags a pipeline update: %s', (_shape, source) => {
680786
expect(findPipelineUpdates(parse('fixture.ts', source))).not.toEqual([]);
681787
});
@@ -730,6 +836,105 @@ describe('Amazon DocumentDB compatibility', () => {
730836
parse('fixture.ts', `interface Doc { $where: Record<string, unknown> }`),
731837
),
732838
).toEqual([]);
839+
/** A dotted `$where` is judged only where the syntax is unambiguous. Not
840+
* claimed either way — Mongoose's document save-condition bag in
841+
* `tenantIsolation.ts` is read and written exactly like this: */
842+
expect(findForbiddenTokens(parse('fixture.ts', `const where = document.$where;`))).toEqual(
843+
[],
844+
);
845+
expect(
846+
findForbiddenTokens(parse('fixture.ts', `document.$where = { tenantId: predicate };`)),
847+
).toEqual([]);
848+
expect(
849+
findForbiddenTokens(
850+
parse('fixture.ts', `document.$where = Object.keys(rest).length > 0 ? rest : undefined;`),
851+
),
852+
).toEqual([]);
853+
/** ...including a non-literal assigned to a filter, which no syntax can tell
854+
* from the bag write; the method sweep and the live run are the backstop. */
855+
expect(findForbiddenTokens(parse('fixture.ts', `filter.$where = predicate;`))).toEqual([]);
856+
/** ...and a bag field that happens to be named like a call forwarder. */
857+
expect(findForbiddenTokens(parse('fixture.ts', `document.$where.call = expected;`))).toEqual(
858+
[],
859+
);
860+
/** Every way of writing the operator itself is an offense: */
861+
expect(
862+
findForbiddenTokens(parse('fixture.ts', `const filter = { $where: 'this.a == 1' };`)),
863+
).not.toEqual([]);
864+
expect(findForbiddenTokens(parse('fixture.ts', `const op = '$where';`))).not.toEqual([]);
865+
expect(
866+
findForbiddenTokens(parse('fixture.ts', `filter['$where'] = 'this.a == 1';`)),
867+
).not.toEqual([]);
868+
/** ...and so is a dotted `$where` that is called or assigned code: */
869+
expect(
870+
findForbiddenTokens(parse('fixture.ts', `filter.$where = 'this.a == 1';`)),
871+
).not.toEqual([]);
872+
expect(
873+
findForbiddenTokens(parse('fixture.ts', 'filter.$where = `this.a == ${value}`;')),
874+
).not.toEqual([]);
875+
expect(
876+
findForbiddenTokens(parse('fixture.ts', `filter.$where = () => this.a == 1;`)),
877+
).not.toEqual([]);
878+
expect(
879+
findForbiddenTokens(parse('fixture.ts', `query.$where(function () { return true; });`)),
880+
).not.toEqual([]);
881+
expect(
882+
findForbiddenTokens(parse('fixture.ts', `Model.find().$where('this.a == 1');`)),
883+
).not.toEqual([]);
884+
expect(
885+
findForbiddenTokens(parse('fixture.ts', `(query.$where)('this.a == 1');`)),
886+
).not.toEqual([]);
887+
expect(
888+
findForbiddenTokens(parse('fixture.ts', `query.$where.call(query, 'this.a == 1');`)),
889+
).not.toEqual([]);
890+
expect(
891+
findForbiddenTokens(parse('fixture.ts', `query.$where.apply(query, ['this.a == 1']);`)),
892+
).not.toEqual([]);
893+
expect(
894+
findForbiddenTokens(parse('fixture.ts', `const w = query.$where.bind(query);`)),
895+
).not.toEqual([]);
896+
expect(findForbiddenTokens(parse('fixture.ts', `this.$where('this.a == 1');`))).not.toEqual(
897+
[],
898+
);
899+
expect(
900+
findForbiddenTokens(parse('fixture.ts', `document.$where('this.a == 1');`)),
901+
).not.toEqual([]);
902+
/** The assignment target and the assigned value may each be wrapped, the
903+
* operator may append, and a forwarder may be reached by element access. */
904+
expect(
905+
findForbiddenTokens(parse('fixture.ts', `(filter.$where as string) = 'this.a == 1';`)),
906+
).not.toEqual([]);
907+
expect(
908+
findForbiddenTokens(parse('fixture.ts', `filter.$where! = 'this.a == 1';`)),
909+
).not.toEqual([]);
910+
expect(
911+
findForbiddenTokens(parse('fixture.ts', `filter.$where += ' && this.b == 2';`)),
912+
).not.toEqual([]);
913+
expect(
914+
findForbiddenTokens(parse('fixture.ts', `filter.$where = <string>'this.a == 1';`)),
915+
).not.toEqual([]);
916+
expect(
917+
findForbiddenTokens(parse('fixture.ts', `query.$where['call'](query, 'this.a == 1');`)),
918+
).not.toEqual([]);
919+
/** Compound assignments of code, and calls through TypeScript's transparent
920+
* wrappers, are the same two shapes spelled differently. */
921+
expect(
922+
findForbiddenTokens(parse('fixture.ts', `filter.$where ??= 'this.a == 1';`)),
923+
).not.toEqual([]);
924+
expect(
925+
findForbiddenTokens(parse('fixture.ts', `filter.$where ||= () => this.a == 1;`)),
926+
).not.toEqual([]);
927+
expect(
928+
findForbiddenTokens(
929+
parse('fixture.ts', `(query.$where as typeof query.$where)('this.a == 1');`),
930+
),
931+
).not.toEqual([]);
932+
expect(findForbiddenTokens(parse('fixture.ts', `query.$where!('this.a == 1');`))).not.toEqual(
933+
[],
934+
);
935+
expect(
936+
findForbiddenTokens(parse('fixture.ts', `(query.$where!).call(query, 'this.a == 1');`)),
937+
).not.toEqual([]);
733938
});
734939

735940
it.each([

0 commit comments

Comments
 (0)