Skip to content

Commit 910eaf7

Browse files
authored
Merge pull request #505 from neo4j/504-flag-to-disable-esaping-labels-and-types
504 flag to disable escaping labels and types
2 parents 40c9a39 + 820d46b commit 910eaf7

File tree

10 files changed

+456
-17
lines changed

10 files changed

+456
-17
lines changed

.changeset/shiny-badgers-reply.md

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@neo4j/cypher-builder": patch
3+
---
4+
5+
New options to disable automatic escaping of labels and relationship types have been added to the `.build` method on clauses, inside the new object `unsafeEscapeOptions`:
6+
7+
- `disableLabelEscaping` (defaults to `false`): If set to true, node labels will not be escaped if unsafe.
8+
- `disableRelationshipTypeEscaping` (defaults to `false`): If set to true, relationship types will not be escaped if unsafe
9+
10+
For example:
11+
12+
```js
13+
const personNode = new Cypher.Node();
14+
const movieNode = new Cypher.Node();
15+
16+
const matchQuery = new Cypher.Match(
17+
new Cypher.Pattern(personNode, {
18+
labels: ["Person"],
19+
properties: {
20+
["person name"]: new Cypher.Literal(`Uneak "Seveer`),
21+
},
22+
})
23+
.related({ type: "ACTED IN" })
24+
.to(movieNode, { labels: ["A Movie"] })
25+
).return(personNode);
26+
27+
const queryResult = matchQuery.build({
28+
unsafeEscapeOptions: {
29+
disableLabelEscaping: true,
30+
disableRelationshipTypeEscaping: true,
31+
},
32+
});
33+
```
34+
35+
This query will generate the following (invalid) Cypher:
36+
37+
```
38+
MATCH (this0:Person { `person name`: "Uneak \"Seveer" })-[:ACTED IN]->(this1:A Movie)
39+
RETURN this0
40+
```
41+
42+
Instead of the default (safe) Cypher:
43+
44+
```cypher
45+
MATCH (this0:Person { `person name`: "Uneak \"Seveer" })-[:`ACTED IN`]->(this1:`A Movie`)
46+
RETURN this0
47+
```
48+
49+
**WARNING:**: Changing these options may lead to code injection and unsafe Cypher.

docs/modules/ROOT/pages/how-to/customize-cypher.adoc

+89
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,92 @@ And the following parameters:
346346
The callback passed into `Raw` is producing the string `this0.prop = $myParam`.
347347
To achieve this, it uses the utility method `utils.compileCypher` and passes the variable `movie` and the `context` parameter, which then returns the string `this0`.
348348
Finally, the custom parameter `$myParam` is returned in the tuple `[cypher, params]`, ensuring that it is available when executing `match.build()`.
349+
350+
351+
== Disable automatic escaping
352+
353+
[WARNING]
354+
====
355+
Changing these options may lead to code injection and unsafe Cypher.
356+
====
357+
358+
Cypher Builder automatically escapes unsafe strings that could lead to code injection. This behavior can be configured using the `unsafeEscapeOptions` parameter in the `.build` method of clauses:
359+
360+
- `disableLabelEscaping` (defaults to `false`): If set to `true`, node labels will not be escaped, even if unsafe.
361+
- `disableRelationshipTypeEscaping` (defaults to `false`): If set to `true`, relationship types will not be escaped, even if unsafe.
362+
363+
For example:
364+
365+
[source, javascript]
366+
----
367+
const personNode = new Cypher.Node();
368+
const movieNode = new Cypher.Node();
369+
370+
const matchQuery = new Cypher.Match(
371+
new Cypher.Pattern(personNode, {
372+
labels: ["Person"],
373+
properties: {
374+
["person name"]: new Cypher.Literal(`Uneak "Seveer`),
375+
},
376+
})
377+
.related({ type: "ACTED IN" })
378+
.to(movieNode, { labels: ["A Movie"] })
379+
).return(personNode);
380+
381+
const queryResult = matchQuery.build({
382+
unsafeEscapeOptions: {
383+
disableLabelEscaping: true,
384+
disableRelationshipTypeEscaping: true,
385+
},
386+
});
387+
----
388+
389+
This query will generate the following (invalid) Cypher:
390+
391+
392+
[source]
393+
----
394+
MATCH (this0:Person { `person name`: "Uneak \"Seveer" })-[:ACTED IN]->(this1:A Movie)
395+
RETURN this0
396+
----
397+
398+
Instead of the default (safe) Cypher:
399+
400+
[source, cypher]
401+
----
402+
MATCH (this0:Person { `person name`: "Uneak \"Seveer" })-[:`ACTED IN`]->(this1:`A Movie`)
403+
RETURN this0
404+
----
405+
406+
=== Manually escaping labels and types
407+
408+
If automatic escaping is disabled, strings used for labels and relationship types must be escaped manually. This can be done using the following utility functions:
409+
410+
* `Cypher.utils.escapeLabel(str)`
411+
* `Cypher.utils.escapeType(str)`
412+
413+
In the previous example, labels and types can be escaped manually to produce valid Cypher:
414+
415+
[source, javascript]
416+
----
417+
const personNode = new Cypher.Node();
418+
const movieNode = new Cypher.Node();
419+
420+
const matchQuery = new Cypher.Match(
421+
new Cypher.Pattern(personNode, {
422+
labels: [Cypher.utils.escapeLabel("Person")],
423+
properties: {
424+
["person name"]: new Cypher.Literal(`Uneak "Seveer`),
425+
},
426+
})
427+
.related({ type: Cypher.utils.escapeType("ACTED IN") })
428+
.to(movieNode, { labels: [Cypher.utils.escapeLabel("A Movie")] })
429+
).return(personNode);
430+
431+
const queryResult = matchQuery.build({
432+
unsafeEscapeOptions: {
433+
disableLabelEscaping: true,
434+
disableRelationshipTypeEscaping: true,
435+
},
436+
});
437+
----

src/Environment.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* limitations under the License.
1818
*/
1919

20+
import type { BuildConfig } from "./Cypher";
2021
import { Param } from "./references/Param";
2122
import type { NamedReference, Variable } from "./references/Variable";
2223

@@ -26,12 +27,15 @@ export type EnvPrefix = {
2627
};
2728

2829
export type EnvConfig = {
29-
labelOperator: ":" | "&";
30-
cypherVersion?: "5";
30+
labelOperator: NonNullable<BuildConfig["labelOperator"]>;
31+
unsafeEscapeOptions: NonNullable<BuildConfig["unsafeEscapeOptions"]>;
32+
cypherVersion: BuildConfig["cypherVersion"];
3133
};
3234

3335
const defaultConfig: EnvConfig = {
3436
labelOperator: ":",
37+
unsafeEscapeOptions: {},
38+
cypherVersion: undefined,
3539
};
3640

3741
/** Hold the internal references of Cypher parameters and variables

src/clauses/Clause.ts

+47-2
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,48 @@ const customInspectSymbol = Symbol.for("nodejs.util.inspect.custom");
2828

2929
/** Config fields for the .build method */
3030
export type BuildConfig = Partial<{
31+
/** Defines the default operator for adding multiple labels in a Pattern. Defaults to `":"`
32+
*
33+
* If the target Cypher is version 5 or above, `"&"` is recommended
34+
*
35+
* @example
36+
* `MATCH (this:Movie:Film)`
37+
* `MATCH (this:Movie&Film)`
38+
*
39+
*/
3140
labelOperator: ":" | "&";
41+
/** Will prefix generated queries with the Cypher version
42+
* @example `CYPHER 5` */
3243
cypherVersion: "5";
44+
/** Prefix variables with given string.
45+
*
46+
* This is useful to avoid name collisions if separate Cypher queries are generated and merged after generating the strings.
47+
* @example `myPrefix_this0`
48+
*
49+
*/
3350
prefix: string;
51+
/** Extra parameters to be added to the result of `.build`. */
3452
extraParams: Record<string, unknown>;
53+
/** Options for disabling automatic escaping of potentially unsafe strings.
54+
*
55+
* **WARNING**: Changing these options may lead to code injection and unsafe Cypher.
56+
*/
57+
unsafeEscapeOptions: Partial<{
58+
/** Disables automatic escaping of node labels.
59+
*
60+
* If disabled, any labels passed should be properly escaped with `utils.escapeLabel`.
61+
*
62+
* **WARNING**: Disabling label escaping may lead to code injection and unsafe Cypher.
63+
*/
64+
disableLabelEscaping: boolean;
65+
/** Disables automatic escaping of relationship types.
66+
*
67+
* If disabled, any types passed should be properly escaped with `utils.escapeType`.
68+
*
69+
* **WARNING**: Disabling type escaping may lead to code injection and unsafe Cypher.
70+
*/
71+
disableRelationshipTypeEscaping: boolean;
72+
}>;
3573
}>;
3674

3775
/** Represents a clause AST node
@@ -41,11 +79,18 @@ export abstract class Clause extends CypherASTNode {
4179
protected nextClause: Clause | undefined;
4280

4381
/** Compiles a clause into Cypher and params */
44-
public build({ prefix, extraParams = {}, labelOperator = ":", cypherVersion }: BuildConfig = {}): CypherResult {
82+
public build({
83+
prefix,
84+
extraParams = {},
85+
labelOperator = ":",
86+
cypherVersion,
87+
unsafeEscapeOptions = {},
88+
}: BuildConfig = {}): CypherResult {
4589
if (this.isRoot) {
4690
const env = this.getEnv(prefix, {
4791
labelOperator,
4892
cypherVersion,
93+
unsafeEscapeOptions,
4994
});
5095
const cypher = this.getCypher(env);
5196

@@ -58,7 +103,7 @@ export abstract class Clause extends CypherASTNode {
58103
}
59104
const root = this.getRoot();
60105
if (root instanceof Clause) {
61-
return root.build({ prefix, extraParams, labelOperator, cypherVersion });
106+
return root.build({ prefix, extraParams, labelOperator, cypherVersion, unsafeEscapeOptions });
62107
}
63108
throw new Error(`Cannot build root: ${root.constructor.name}`);
64109
}

src/pattern/PartialPattern.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import type { Expr } from "../types";
2626
import { compileCypherIfExists } from "../utils/compile-cypher-if-exists";
2727
import { NestedPattern, type NodePattern, type Pattern, type RelationshipPattern } from "./Pattern";
2828
import { PatternElement } from "./PatternElement";
29-
import { labelsToString } from "./labels-to-string";
29+
import { typeToString } from "./labels-to-string";
3030

3131
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
3232
export interface PartialPattern extends WithWhere {}
@@ -99,7 +99,7 @@ export class PartialPattern extends PatternElement {
9999

100100
private getTypeStr(env: CypherEnvironment): string {
101101
if (this.type) {
102-
return labelsToString(this.type, env);
102+
return typeToString(this.type, env);
103103
}
104104
return "";
105105
}

src/pattern/Pattern.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -517,4 +517,16 @@ describe("Patterns", () => {
517517

518518
expect(queryResult.params).toMatchInlineSnapshot(`{}`);
519519
});
520+
521+
test("Pattern with empty strings as labels and types", () => {
522+
const a = new Cypher.Node();
523+
const b = new Cypher.Node();
524+
const rel = new Cypher.Relationship();
525+
526+
const query = new TestClause(new Cypher.Pattern(a, { labels: [""] }).related(rel, { type: "" }).to(b));
527+
const queryResult = query.build();
528+
expect(queryResult.cypher).toMatchInlineSnapshot(`"(this0)-[this1]->(this2)"`);
529+
530+
expect(queryResult.params).toMatchInlineSnapshot(`{}`);
531+
});
520532
});

src/pattern/labels-to-string.ts

+15-9
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,26 @@ import type { CypherEnvironment } from "../Environment";
2121
import { LabelExpr } from "../expressions/labels/label-expressions";
2222
import { addLabelToken } from "../utils/add-label-token";
2323
import { asArray } from "../utils/as-array";
24-
import { escapeLabel } from "../utils/escape";
24+
import { escapeLabel, escapeType } from "../utils/escape";
2525

2626
export function labelsToString(labels: string | string[] | LabelExpr, env: CypherEnvironment): string {
2727
if (labels instanceof LabelExpr) {
28-
const labelsStr = labels.getCypher(env);
29-
if (!labelsStr) {
30-
return "";
31-
}
3228
return addLabelToken(env.config.labelOperator, labels.getCypher(env));
3329
} else {
34-
const escapedLabels = asArray(labels).map(escapeLabel);
35-
if (escapedLabels.length === 0) {
36-
return "";
37-
}
30+
const shouldEscape = !env.config.unsafeEscapeOptions.disableLabelEscaping;
31+
const escapedLabels = shouldEscape ? asArray(labels).map(escapeLabel) : asArray(labels);
32+
3833
return addLabelToken(env.config.labelOperator, ...escapedLabels);
3934
}
4035
}
36+
37+
export function typeToString(type: string | LabelExpr, env: CypherEnvironment): string {
38+
if (type instanceof LabelExpr) {
39+
return addLabelToken(env.config.labelOperator, type.getCypher(env));
40+
} else {
41+
const shouldEscape = !env.config.unsafeEscapeOptions.disableRelationshipTypeEscaping;
42+
const escapedType = shouldEscape ? escapeType(type) : type;
43+
44+
return addLabelToken(env.config.labelOperator, escapedType);
45+
}
46+
}

src/references/Variable.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
* limitations under the License.
1818
*/
1919

20-
import { PropertyRef } from "./PropertyRef";
20+
import type { CypherEnvironment } from "../Environment";
2121
import { ListIndex } from "../expressions/list/ListIndex";
2222
import type { Expr } from "../types";
23-
import type { CypherEnvironment } from "../Environment";
2423
import { escapeVariable } from "../utils/escape";
24+
import { PropertyRef } from "./PropertyRef";
2525

2626
/** Represents a variable
2727
* @group Variables

src/utils/stringify-object.test.ts

+9
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,13 @@ describe("stringifyObject", () => {
3939

4040
expect(result).toBe(`{ nobody: expects }`);
4141
});
42+
43+
test("escape keys", () => {
44+
const result = stringifyObject({
45+
["this name"]: "this",
46+
that: `"that"`,
47+
});
48+
49+
expect(result).toBe(`{ \`this name\`: this, that: "that" }`);
50+
});
4251
});

0 commit comments

Comments
 (0)