diff --git a/src/__tests__/__snapshots__/index.test.ts.snap b/src/__tests__/__snapshots__/index.test.ts.snap index 294ac83..e26874a 100644 --- a/src/__tests__/__snapshots__/index.test.ts.snap +++ b/src/__tests__/__snapshots__/index.test.ts.snap @@ -11,6 +11,7 @@ Array [ "Fn::And", "0", ], + "value": "\\"\\"", }, ] `; @@ -25,6 +26,7 @@ Array [ "C", "Fn::And", ], + "value": "\\"\\"", }, ] `; @@ -86,6 +88,7 @@ Array [ "Fn::Not", "0", ], + "value": "\\"\\"", }, ] `; @@ -100,6 +103,7 @@ Array [ "C", "Fn::Not", ], + "value": "\\"\\"", }, ] `; @@ -119,6 +123,7 @@ Array [ "Fn::Or", "0", ], + "value": "\\"\\"", }, ] `; @@ -133,6 +138,7 @@ Array [ "C", "Fn::Or", ], + "value": "\\"\\"", }, ] `; @@ -148,6 +154,7 @@ Array [ "And", "0", ], + "value": "\\"\\"", }, ] `; @@ -162,6 +169,7 @@ Array [ "C", "And", ], + "value": "\\"\\"", }, ] `; @@ -223,6 +231,7 @@ Array [ "Not", "0", ], + "value": "\\"\\"", }, ] `; @@ -237,6 +246,7 @@ Array [ "C", "Not", ], + "value": "\\"\\"", }, ] `; @@ -256,6 +266,7 @@ Array [ "Or", "0", ], + "value": "\\"\\"", }, ] `; @@ -270,6 +281,7 @@ Array [ "C", "Or", ], + "value": "\\"\\"", }, ] `; @@ -390,6 +402,7 @@ Array [ "BucketName", "Fn::ImportValue", ], + "value": "[\\"\\",[\\"\\"],\\"\\"]", }, ] `; @@ -406,6 +419,7 @@ Array [ "BucketName", "Fn::ImportValue", ], + "value": "[[],\\"\\"]", }, ] `; @@ -422,6 +436,7 @@ Array [ "BucketName", "Fn::ImportValue", ], + "value": "{\\"a\\":\\"\\"}", }, ] `; @@ -438,6 +453,7 @@ Array [ "BucketName", "Fn::ImportValue", ], + "value": "[\\"\\"]", }, ] `; @@ -454,6 +470,7 @@ Array [ "BucketName", "ImportValue", ], + "value": "[\\"\\",[\\"\\"],\\"\\"]", }, ] `; @@ -470,6 +487,7 @@ Array [ "BucketName", "ImportValue", ], + "value": "[[],\\"\\"]", }, ] `; @@ -486,6 +504,7 @@ Array [ "BucketName", "ImportValue", ], + "value": "{\\"a\\":\\"\\"}", }, ] `; @@ -502,6 +521,7 @@ Array [ "BucketName", "ImportValue", ], + "value": "[\\"\\"]", }, ] `; @@ -518,6 +538,7 @@ Array [ "Root", "Mappings", ], + "value": "\\"\\"", }, ] `; @@ -532,6 +553,7 @@ Array [ "a", "b", ], + "value": "\\"\\"", }, ] `; @@ -552,6 +574,7 @@ Array [ "Root", "Outputs", ], + "value": "\\"\\"", }, ] `; @@ -566,6 +589,7 @@ Array [ "O", "Description", ], + "value": "{}", }, ] `; @@ -581,6 +605,7 @@ Array [ "Export", "Name", ], + "value": "{}", }, ] `; @@ -595,6 +620,7 @@ Array [ "O", "Value", ], + "value": "{}", }, ] `; @@ -819,6 +845,7 @@ Array [ "BucketName", "Fn::Join", ], + "value": "\\"\\"", }, ] `; @@ -836,6 +863,7 @@ Array [ "Fn::Join", "0", ], + "value": "[]", }, Object { "message": "must be a List, got \\"\\"", @@ -848,6 +876,7 @@ Array [ "Fn::Join", "1", ], + "value": "\\"\\"", }, ] `; @@ -896,6 +925,7 @@ Array [ "BucketName", "Join", ], + "value": "\\"\\"", }, ] `; @@ -913,6 +943,7 @@ Array [ "Join", "0", ], + "value": "[]", }, Object { "message": "must be a List, got \\"\\"", @@ -925,6 +956,7 @@ Array [ "Join", "1", ], + "value": "\\"\\"", }, ] `; @@ -973,6 +1005,7 @@ Array [ "BucketName", "Ref", ], + "value": "[\\"A\\"]", }, ] `; @@ -1005,6 +1038,7 @@ Array [ "BucketName", "Ref", ], + "value": "[\\"A\\"]", }, ] `; @@ -1070,6 +1104,7 @@ Array [ "Fn::Sub", "0", ], + "value": "{}", }, Object { "message": "must be an Object, got \\"\${A}\\"", @@ -1082,6 +1117,7 @@ Array [ "Fn::Sub", "1", ], + "value": "\\"\${A}\\"", }, ] `; @@ -1179,6 +1215,7 @@ Array [ "Sub", "0", ], + "value": "{}", }, Object { "message": "must be an Object, got \\"\${A}\\"", @@ -1191,6 +1228,7 @@ Array [ "Sub", "1", ], + "value": "\\"\${A}\\"", }, ] `; @@ -1411,7 +1449,7 @@ Array [ exports[`lint with parameters string values invalid default 1`] = ` Array [ Object { - "message": "must be a Number, got Ref ExpirationInDays", + "message": "must be a Number, got {\\"Ref\\":\\"ExpirationInDays\\"}", "path": Array [ "Root", "Resources", @@ -1422,6 +1460,7 @@ Array [ "0", "ExpirationInDays", ], + "value": "{\\"Ref\\":\\"ExpirationInDays\\"}", }, ] `; @@ -1429,7 +1468,7 @@ Array [ exports[`lint with parameters string values invalid value 1`] = ` Array [ Object { - "message": "must be a Number, got Ref ExpirationInDays", + "message": "must be a Number, got {\\"Ref\\":\\"ExpirationInDays\\"}", "path": Array [ "Root", "Resources", @@ -1440,6 +1479,7 @@ Array [ "0", "ExpirationInDays", ], + "value": "{\\"Ref\\":\\"ExpirationInDays\\"}", }, ] `; diff --git a/src/index.ts b/src/index.ts index e831f5c..07a2331 100644 --- a/src/index.ts +++ b/src/index.ts @@ -80,5 +80,12 @@ export function lint(template: string, parameters: object = {}) { ]; const validate = new Walker(validators); validate.Root(input); - return errors; + + return _.map(errors, (e) => { + if ('value' in e) { + return {...e, message: `${e.message}, got ${e.value}`}; + } else { + return e; + } + }); } diff --git a/src/types.ts b/src/types.ts index d42838b..0d85827 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,6 +4,7 @@ export type Error = { path: Path, message: string, source?: string, + value?: string, }; export class ResourceSpecificationError extends Error { diff --git a/src/validate.ts b/src/validate.ts index bfe25d1..79dff30 100644 --- a/src/validate.ts +++ b/src/validate.ts @@ -148,7 +148,7 @@ export function object(path: Path, } return true; } else { - errors.push({ path, message: `must be an Object, got ${JSON.stringify(o)}` }); + errors.push({ path, message: `must be an Object`, value: JSON.stringify(o) }); return false; } } @@ -190,7 +190,7 @@ export function list( } return true; } else { - errors.push({ path, message: `must be a List, got ${JSON.stringify(o)}` }); + errors.push({ path, message: `must be a List`, value: JSON.stringify(o) }); return false; } } @@ -208,14 +208,14 @@ export function string(path: Path, o: any, errors: Error[], allowedValues: strin if(_.find(allowedValues, (v) => new RegExp(v, 'i').test(str))) { return true; } else { - errors.push({ path, message: `must be one of ${allowedValues.join(', ')}, got ${JSON.stringify(o)}` }); + errors.push({ path, message: `must be one of ${allowedValues.join(', ')}`, value: JSON.stringify(o) }); return false; } } else { return true; } } else { - errors.push({ path, message: `must be a String, got ${JSON.stringify(o)}` }); + errors.push({ path, message: `must be a String`, value: JSON.stringify(o) }); return false; } } @@ -228,7 +228,7 @@ export function number(path: Path, o: any, errors: Error[]): boolean { } else if (isStringNumber(o)) { return true; } else { - errors.push({ path, message: `must be a Number, got ${o}` }); + errors.push({ path, message: `must be a Number`, value: JSON.stringify(o) }); return false; } } @@ -241,7 +241,7 @@ export function boolean(path: Path, o: any, errors: Error[]): boolean { } else if (_.isBoolean(o)) { return true; } else { - errors.push({ path, message: `must be a Boolean, got ${JSON.stringify(o)}` }); + errors.push({ path, message: `must be a Boolean`, value: JSON.stringify(o) }); return false; } } @@ -252,7 +252,7 @@ export function or(...fns: ValidationFn[]): ValidationFn { const success = _.some(fns, (fn) => fn(path, value, errs)); if (!success) { const message = _.join(_.map(errs, (e) => e.message), ' or '); - errors.push({ path, message }); + errors.push({ path, message, value: JSON.stringify(value) }); } return success; } diff --git a/src/validators/IAMPolicyDocumentValidator.ts b/src/validators/IAMPolicyDocumentValidator.ts index 3300937..6734117 100644 --- a/src/validators/IAMPolicyDocumentValidator.ts +++ b/src/validators/IAMPolicyDocumentValidator.ts @@ -2,6 +2,7 @@ import * as _ from 'lodash'; import { Validator, ValidationFn } from '../validate'; import * as validate from '../validate'; import { Path, Error } from '../types'; +import { withSuggestion } from '../util'; const listOf = (fn: ValidationFn): ValidationFn => { return (path: Path, value: any, errors: Error[]) => { @@ -15,15 +16,124 @@ const stringOf = (allowedValues: string[]): ValidationFn => { } } +// https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition_operators.html +const operators = [ + 'StringEquals', + 'StringNotEquals', + 'StringEqualsIgnoreCase', + 'StringNotEqualsIgnoreCase', + 'StringLike', + 'StringNotLike', + 'NumericEquals', + 'NumericNotEquals', + 'NumericLessThan', + 'NumericLessThanEquals', + 'NumericGreaterThan', + 'NumericGreaterThanEquals', + 'DateEquals', + 'DateNotEquals', + 'DateLessThan', + 'DateLessThanEquals', + 'DateGreaterThan', + 'DateGreaterThanEquals', + 'Bool', + 'Null', + 'BinaryEquals', + 'IpAddress', + 'NotIpAddress', + 'ArnEquals', + 'ArnLike', + 'ArnNotEquals', + 'ArnNotLike', +]; + +// https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html +const conditionKeys = [ + 'aws:CurrentTime', + 'aws:EpochTime', + 'aws:MultiFactorAuthAge', + 'aws:MultiFactorAuthPresent', + 'aws:SecureTransport', + 'aws:UserAgent', + 'aws:PrincipalOrgID', + /^aws:PrincipalTag\//, + 'aws:PrincipalType', + 'aws:Referer', + 'aws:RequestedRegion', + /^aws:RequestTag\//, + 'aws:SourceAccount', + 'aws:SourceArn', + 'aws:SourceIp', + 'aws:SourceVpc', + 'aws:SourceVpce', + 'aws:TagKeys', + 'aws:TokenIssueTime', + 'aws:userid', + 'aws:username', + /^s3:/, +]; + +const validateCondition = (path: Path, condition: any, errors: Error[]): boolean => { + if (validate.object(path, condition, errors)) { + _.forEach(condition, (value, key) => { + path = path.concat(key); + if (!_.includes(operators, key.toString())) { + const message = withSuggestion( + `key must be one of ${operators.join(', ')}, got ${key}`, + operators, + key + ); + errors.push({ path, message }); + } + if (validate.object(path, value, errors)) { + _.forEach(value, (conditionValue, conditionKey) => { + if(!_.some(conditionKeys, (k) => conditionKey.match(k))) { + const keys = _.map(conditionKeys, (c) => String(c).replace(/[^A-Za-z:]/g, '')); + const message = withSuggestion( + `key must be one of ${keys.join(', ')}, got ${conditionKey}`, + keys, + conditionKey + ); + errors.push({ + path: path.concat(conditionKey), + message: message + }); + validate.or(validate.string, listOf(validate.string))( + path.concat(conditionKey), + conditionValue, + errors + ); + } + }); + } + }); + return true; + } else { + return false; + } +} + +const validatePrincipal = (path: Path, value: any, errors: Error[]): boolean => { + // https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html + const spec = { + AWS: [validate.optional, validate.or(validate.string, listOf(validate.string))], + Service: [validate.optional, validate.or(validate.string, listOf(validate.string))], + Federated: [validate.optional, validate.or(validate.string, listOf(validate.string))], + CanonicalUser: [validate.optional, validate.or(validate.string, listOf(validate.string))], + } + return validate.or(validate.string, (p, v, e) => validate.object(p, v, e, spec))(path, value, errors); +} + const validateStatement = (path: Path, value: any, errors: Error[]): boolean => { const spec = { Sid: [validate.optional, validate.string], Effect: [validate.required, stringOf(['Allow', 'Deny'])], - // Principal can not be specified for inline policies - Principal: [validate.optional, validate.or(validate.string, listOf(validate.string))], - Action: [validate.required, validate.or(validate.string, listOf(validate.string))], + // TODO Principal can not be specified for inline policies + Principal: [validate.optional, validatePrincipal], + Action: [validate.optional, validate.or(validate.string, listOf(validate.string))], + NotAction: [validate.optional, validate.or(validate.string, listOf(validate.string))], Resource: [validate.optional, validate.or(validate.string, listOf(validate.string))], - Condition: [validate.optional, validate.object], + Condition: [validate.optional, validateCondition], }; return validate.object(path, value, errors, spec); } diff --git a/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts b/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts index f92068f..d7ba044 100644 --- a/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts +++ b/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts @@ -1,7 +1,10 @@ import { lint } from '../../index'; describe('IAMPolicyDocumentValidator', () => { - test('valid', () => { + test.each([ + [{ Bool: { 'aws:SecureTransport': true } }], + [{ StringEquals: { 'aws:RequestTag/foo': ['bar'] } }], + ])('valid %s', (condition) => { const template = JSON.stringify({ Resources: { Role: { @@ -19,7 +22,7 @@ describe('IAMPolicyDocumentValidator', () => { Effect: 'Allow', Principal: '', Resource: '', - Condition: {} + Condition: condition } ] } @@ -30,8 +33,7 @@ describe('IAMPolicyDocumentValidator', () => { }); expect(lint(template)).toEqual([]); }); - - test('valid, with lists', () => { + test('valid, with resource list', () => { const template = JSON.stringify({ Resources: { Role: { @@ -44,12 +46,10 @@ describe('IAMPolicyDocumentValidator', () => { Version: '', Statement: [ { - Sid: '', Action: '', Effect: 'Deny', - Principal: [''], + Principal: '', Resource: [''], - Condition: {} } ] } @@ -84,6 +84,62 @@ describe('IAMPolicyDocumentValidator', () => { expect(lint(template)).toMatchSnapshot(); }); + test('invalid conditional', () => { + const template = JSON.stringify({ + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: {}, + Policies: [{ + PolicyName: '', + PolicyDocument: { + Version: '', + Statement: [ + { + Action: '', + Effect: 'Allow', + Resource: '', + Condition: { Foo: { 'aws:SecureTransport': true } } + } + ] + } + }] + } + } + } + }); + expect(lint(template)).toMatchSnapshot(); + }); + + test('invalid condition key', () => { + const template = JSON.stringify({ + Resources: { + Role: { + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: {}, + Policies: [{ + PolicyName: '', + PolicyDocument: { + Version: '', + Statement: [ + { + Action: '', + Effect: 'Allow', + Resource: '', + Condition: { Bool: { 'cats': 'dogs' } } + } + ] + } + }] + } + } + } + }); + expect(lint(template)).toMatchSnapshot(); + }); + test('invalid types', () => { const template = JSON.stringify({ Resources: { @@ -100,7 +156,7 @@ describe('IAMPolicyDocumentValidator', () => { Sid: [], Action: [], Effect: [], - Principal: {}, + Principal: [], Resource: {}, Condition: '', } @@ -127,12 +183,9 @@ describe('IAMPolicyDocumentValidator', () => { Version: '', Statement: [ { - Sid: '', Action: '', Effect: 'foo', - Principal: '', Resource: '', - Condition: {} } ] } diff --git a/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap b/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap index f7b83f4..6c76bd2 100644 --- a/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap +++ b/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap @@ -1,5 +1,48 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`IAMPolicyDocumentValidator invalid condition key 1`] = ` +Array [ + Object { + "message": "key must be one of aws:CurrentTime, aws:EpochTime, aws:MultiFactorAuthAge, aws:MultiFactorAuthPresent, aws:SecureTransport, aws:UserAgent, aws:PrincipalOrgID, aws:PrincipalTag, aws:PrincipalType, aws:Referer, aws:RequestedRegion, aws:RequestTag, aws:SourceAccount, aws:SourceArn, aws:SourceIp, aws:SourceVpc, aws:SourceVpce, aws:TagKeys, aws:TokenIssueTime, aws:userid, aws:username, s:, got cats", + "path": Array [ + "Root", + "Resources", + "Role", + "Properties", + "Policies", + "0", + "PolicyDocument", + "Statement", + "0", + "Condition", + "Bool", + "cats", + ], + }, +] +`; + +exports[`IAMPolicyDocumentValidator invalid conditional 1`] = ` +Array [ + Object { + "message": "key must be one of StringEquals, StringNotEquals, StringEqualsIgnoreCase, StringNotEqualsIgnoreCase, StringLike, StringNotLike, NumericEquals, NumericNotEquals, NumericLessThan, NumericLessThanEquals, NumericGreaterThan, NumericGreaterThanEquals, DateEquals, DateNotEquals, DateLessThan, DateLessThanEquals, DateGreaterThan, DateGreaterThanEquals, Bool, Null, BinaryEquals, IpAddress, NotIpAddress, ArnEquals, ArnLike, ArnNotEquals, ArnNotLike, got Foo, did you mean Bool?", + "path": Array [ + "Root", + "Resources", + "Role", + "Properties", + "Policies", + "0", + "PolicyDocument", + "Statement", + "0", + "Condition", + "Foo", + ], + }, +] +`; + exports[`IAMPolicyDocumentValidator invalid effect 1`] = ` Array [ Object { @@ -16,6 +59,7 @@ Array [ "0", "Effect", ], + "value": "\\"foo\\"", }, ] `; @@ -36,6 +80,7 @@ Array [ "0", "Sid", ], + "value": "[]", }, Object { "message": "must be a String, got []", @@ -51,9 +96,10 @@ Array [ "0", "Effect", ], + "value": "[]", }, Object { - "message": "must be a String, got {} or must be a List, got {}", + "message": "must be a String or must be an Object, got []", "path": Array [ "Root", "Resources", @@ -66,9 +112,10 @@ Array [ "0", "Principal", ], + "value": "[]", }, Object { - "message": "must be a String, got {} or must be a List, got {}", + "message": "must be a String or must be a List, got {}", "path": Array [ "Root", "Resources", @@ -81,6 +128,7 @@ Array [ "0", "Resource", ], + "value": "{}", }, Object { "message": "must be an Object, got \\"\\"", @@ -96,6 +144,7 @@ Array [ "0", "Condition", ], + "value": "\\"\\"", }, ] `; @@ -117,20 +166,5 @@ Array [ "Effect", ], }, - Object { - "message": "is required", - "path": Array [ - "Root", - "Resources", - "Role", - "Properties", - "Policies", - "0", - "PolicyDocument", - "Statement", - "0", - "Action", - ], - }, ] `;