Skip to content

Commit

Permalink
fix: IaC path parsing
Browse files Browse the repository at this point in the history
Fix a bug in which IaC resource paths (more context below) was naively
split by dot, without distinguishing between separator dots and dots
inside string literals.

For example: "foo.bar.baz" was split into 3 words correctly, but
"metadata.annotations['container.apparmor.security.beta.kubernetes.io/web']"
was not.

The "path" / "cloud config path" is a sort of address used to identify
locations in files where vulnerabilities reside. It is very similar to a
jsonpath. The user receives it as an array of components, which in the
future can be useful for building tooling around, e.g. the upcoming CLI
ignores.

The code that evaluates policies against source files is also
responsible for returning a representation of this path, which it does
as a dot-separated string.
  • Loading branch information
Craig Furman committed Jul 29, 2021
1 parent d8d41a4 commit 8566273
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 1 deletion.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -114,6 +114,7 @@
"open": "^7.0.3",
"ora": "5.4.0",
"os-name": "^3.0.0",
"pegjs": "^0.10.0",
"promise-queue": "^2.2.5",
"proxy-from-env": "^1.0.0",
"rimraf": "^2.6.3",
Expand Down
35 changes: 35 additions & 0 deletions src/cli/commands/test/iac-local-execution/parsers/path.ts
@@ -0,0 +1,35 @@
import * as peg from 'pegjs';

const grammar = `
start
= element+
element
= component:component "."? { return component; }
component
= $(identifier index?)
identifier
= $([^'"\\[\\]\\.]+)
index
= $("[" ['"]? [^'"\\]]+ ['"]? "]")
`;

export const parsePath = createPathParser();

function createPathParser(): (expr: string) => string[] {
const parser = peg.generate(grammar);
return (expr: string) => {
try {
return parser.parse(expr);
} catch (e) {
// I haven't actually been able to write a testcase that triggers this
// code path, but I've included it anyway as a fallback to allow users to
// keep using the CLI even if this does occur. Their paths might look
// strange, but that's better than nothing.
return expr.split('.');
}
};
}
Expand Up @@ -7,6 +7,7 @@ import {
PolicyMetadata,
TestMeta,
} from './types';
import { parsePath } from './parsers/path';
import * as path from 'path';
import { SEVERITY } from '../../../../lib/snyk-test/common';
import { IacProjectType } from '../../../../lib/iac/constants';
Expand Down Expand Up @@ -55,7 +56,7 @@ function formatScanResult(
const formattedIssues = scanResult.violatedPolicies.map((policy) => {
const cloudConfigPath =
scanResult.docId !== undefined
? [`[DocId: ${scanResult.docId}]`].concat(policy.msg.split('.'))
? [`[DocId: ${scanResult.docId}]`].concat(parsePath(policy.msg))
: policy.msg.split('.');

const flagsRequiringLineNumber = [
Expand Down
19 changes: 19 additions & 0 deletions test/jest/unit/iac-unit-tests/path-parser.spec.ts
@@ -0,0 +1,19 @@
import { parsePath } from '../../../../src/cli/commands/test/iac-local-execution/parsers/path';

describe('parsing cloudConfigPath', () => {
it.each([
['foo', ['foo']],
['foo.bar.baz', ['foo', 'bar', 'baz']],
['foo_1._bar2.baz3_', ['foo_1', '_bar2', 'baz3_']],
['foo.bar[abc].baz', ['foo', 'bar[abc]', 'baz']],
['foo.bar[abc.def].baz', ['foo', 'bar[abc.def]', 'baz']],
["foo.bar['abc.def'].baz", ['foo', "bar['abc.def']", 'baz']],
['foo.bar["abc.def"].baz', ['foo', 'bar["abc.def"]', 'baz']],
["foo.bar['abc/def'].baz", ['foo', "bar['abc/def']", 'baz']],
["foo.bar['abcdef'].baz", ['foo', "bar['abcdef']", 'baz']],
["bar['abc.def']", ["bar['abc.def']"]],
["fo%o.bar['ab$c/def'].baz", ['fo%o', "bar['ab$c/def']", 'baz']],
])('%s', (input, expected) => {
expect(parsePath(input)).toEqual(expected);
});
});

0 comments on commit 8566273

Please sign in to comment.