Skip to content

Commit

Permalink
[Enhancement] Add typo checker and correct errors (#464)
Browse files Browse the repository at this point in the history
* Add typos and correct files

* fix typos manually and update config

* added attribution

* new line at the bottom of config

* clean up action boilerplate

* new line

* update config for dne

* bad resolution
  • Loading branch information
dannyvassallo authored Feb 16, 2024
1 parent 4f88655 commit 2b5d57d
Show file tree
Hide file tree
Showing 28 changed files with 104 additions and 60 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
args: -- -D warnings

typocheck:
name: Spell Check with Typos
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: crate-ci/typos@master
with:
files: .
config: ./_typos.toml

fuzz:
strategy:
matrix:
Expand Down
24 changes: 24 additions & 0 deletions ATTRIBUTION
Original file line number Diff line number Diff line change
Expand Up @@ -1741,3 +1741,27 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

--

crate-ci/typos

Copyright (c) Individual contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
10 changes: 10 additions & 0 deletions _typos.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[default.extend-words]
# Don't correct "archtype" to "archetype"
archtype = "archtype"
# Don't flag ure in `FAIL`ure as a misspelling
ure = "ure"
# Don't flag dne as a misspelling
dne = "dne"
[default.extend-identifiers]
# Ignore ID in evaluate_tests.rs
fooCounterTaskDef49BA9021 = "fooCounterTaskDef49BA9021"
2 changes: 1 addition & 1 deletion docs/CLAUSES.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Properties.BucketName != /(?i)encrypted/

```
# ReadCapacityUnits must be less than or equal to 5000
Properties.ProvisionedThroughtput.ReadCapacityUnits <= 5000
Properties.ProvisionedThroughput.ReadCapacityUnits <= 5000
```

## Operators
Expand Down
6 changes: 3 additions & 3 deletions docs/COMPLEX_COMPOSITION.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ rule check_parameter_validity when check_resource_type_and_parameter {
}
}
rule check_ip_procotol_and_port_range_validity when check_parameter_validity {
rule check_ip_protocol_and_port_range_validity when check_parameter_validity {
let ports = InputParameters.TcpBlockedPorts[*]
#
Expand Down Expand Up @@ -126,7 +126,7 @@ rule check_ip_procotol_and_port_range_validity when check_parameter_validity {
}
```

`check_parameter_validity` is conditionally dependent on `check_resource_type_and_parameter` and `check_ip_procotol_and_port_range_validity` is conditionally dependent on `check_parameter_validity`. Below is a CMDB configuration item which conforms to the above rules:
`check_parameter_validity` is conditionally dependent on `check_resource_type_and_parameter` and `check_ip_protocol_and_port_range_validity` is conditionally dependent on `check_parameter_validity`. Below is a CMDB configuration item which conforms to the above rules:

```yaml
---
Expand Down Expand Up @@ -263,7 +263,7 @@ cfn-guard validate --data /path/to/dataDirectory --rules /path/to/ruleDirectory

Where `/path/to/dataDirectory` has one or more data files and `/path/to/ruleDirectory` has one or more rules files.

Consider you are writing rules to check if various resources defined in multiple CloudFormation templates have appropriate property assignments which guarantees encryption at rest. For ease of searchability and maintainability you can decide to have rules for checking encryption at rest in each resource in separate files, e.g. `s3_bucket_encryption.guard`, `ec2_volume_encryption.guard`, `rds_dbinstance_encrytion.guard` and so on all inside one directory, `~/GuardRules/encryption_at_rest`. You have all your CloudFormation templates that you need to validate under `~/CloudFormation/templates`. The command will be as follows:
Consider you are writing rules to check if various resources defined in multiple CloudFormation templates have appropriate property assignments which guarantees encryption at rest. For ease of searchability and maintainability you can decide to have rules for checking encryption at rest in each resource in separate files, e.g. `s3_bucket_encryption.guard`, `ec2_volume_encryption.guard`, `rds_dbinstance_encryption.guard` and so on all inside one directory, `~/GuardRules/encryption_at_rest`. You have all your CloudFormation templates that you need to validate under `~/CloudFormation/templates`. The command will be as follows:

```bash
cfn-guard validate --data ~/CloudFormation/templates --rules ~/GuardRules/encryption_at_rest
Expand Down
14 changes: 7 additions & 7 deletions docs/CONTEXTAWARE_EVALUATIONS_AND_LOOPS.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ configuration:
The following rule when tested against the template above results in an error, as it makes use of an incorrect assumption of leveraging the implicit `this`:

```
rule check_ip_procotol_and_port_range_validity
rule check_ip_protocol_and_port_range_validity
{
#
# select all ipPermission instances that can be reached by ANY IP address
Expand Down Expand Up @@ -293,7 +293,7 @@ Clause #3 FAIL(Block[Location[file:any_ip_ingress_check.guard, line:17, colu
The engine indicates that its attempt to retrieve a property `InputParameters.TcpBlockedPorts[*]` on the value `/configuration/ipPermissions/0`, `/configuration/ipPermissions/1` failed. To understand this better, let us rewrite the rule above using this explicitly referenced.

```
rule check_ip_procotol_and_port_range_validity
rule check_ip_protocol_and_port_range_validity
{
#
# select all ipPermission instances that can be reached by ANY IP address
Expand Down Expand Up @@ -322,7 +322,7 @@ rule check_ip_procotol_and_port_range_validity
}
```

`this` next to `InputParameters` references to each value contained inside variable `any_ip_permissions`. The query assigned to the variable selects `configuration.ipPermissions` values that match. The error indicates that we are attempting to retrieve `InputParamaters` in this context but `InputParameters` was on the root context.
`this` next to `InputParameters` references to each value contained inside variable `any_ip_permissions`. The query assigned to the variable selects `configuration.ipPermissions` values that match. The error indicates that we are attempting to retrieve `InputParameters` in this context but `InputParameters` was on the root context.

The same is true for the inner block:

Expand All @@ -347,7 +347,7 @@ The same is true for the inner block:
Using variables to explicitly assign and reference them. First `InputParameter.TcpBlockedPorts`, is part of input/root context. We should therefore move this out of the inner block and assign it explicitly:

```
rule check_ip_procotol_and_port_range_validity
rule check_ip_protocol_and_port_range_validity
{
let ports = InputParameters.TcpBlockedPorts[*]
# ... cut off for illustrating change
Expand All @@ -357,7 +357,7 @@ rule check_ip_procotol_and_port_range_validity
You can then refer to this variable explicitly:

```
rule check_ip_procotol_and_port_range_validity
rule check_ip_protocol_and_port_range_validity
{
#
# Important: it would be an ERROR to just assign InputParameters.TcpBlockedPorts
Expand Down Expand Up @@ -400,7 +400,7 @@ Let’s do the same for inner `this` references within `%ports`:
This still does not completely fix all errors, the loop inside ports still has an incorrect reference. We need to fix that was well.

```
rule check_ip_procotol_and_port_range_validity
rule check_ip_protocol_and_port_range_validity
{
#
# Important: it would be an ERROR to just assign InputParameters.TcpBlockedPorts
Expand Down Expand Up @@ -461,7 +461,7 @@ Let us try another run with this file against the payload `cfn-guard validate -r
```bash
Summary Report Overall File Status = PASS
PASS/SKIP rules
check_ip_procotol_and_port_range_validity PASS
check_ip_protocol_and_port_range_validity PASS
```

Does it work for `FAIL`ure case? Let us give it a try with the following payload change
Expand Down
2 changes: 1 addition & 1 deletion docs/FUNCTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ The following rules show different ways we can use the count function.
- One queries a struct, and counts the number of properties.
- The second queries a list object, and counts the elements in the list
- The third queries for all resources that are s3 buckets and have a PublicAcessBlockConfiguration property
- The third queries for all resources that are s3 buckets and have a PublicAccessBlockConfiguration property
#### Example
Expand Down
2 changes: 1 addition & 1 deletion docs/KNOWN_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ or

The block form iterates over all `AWS::ApiGateway::RestApi` resources found in the input. The first form short circuits and returns immediately after the first resource failure.

> **Workaround**: use the block form to traverse all values to show all resource failures and not just the first one that failed. We are tracking to resolve this issue. 2. Need `when` guards with filter expressions- When a query uses filters like `Resources.*[ Type == 'AWS::ApiGateway::RestApi' ]`, if there are no `ApiGatway` resources, then Guard will fail the clause today when performing the check
> **Workaround**: use the block form to traverse all values to show all resource failures and not just the first one that failed. We are tracking to resolve this issue. 2. Need `when` guards with filter expressions- When a query uses filters like `Resources.*[ Type == 'AWS::ApiGateway::RestApi' ]`, if there are no `ApiGateway` resources, then Guard will fail the clause today when performing the check
```
%api_gws.Properties.EndpointConfiguration.Types[*] == "PRIVATE"
Expand Down
6 changes: 3 additions & 3 deletions docs/QUERY_AND_FILTERING.md
Original file line number Diff line number Diff line change
Expand Up @@ -489,14 +489,14 @@ Resources:
ecsTask:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn: 'arn:aws:....'
ExecutionRoleArn: 'arn:aws:...'
ecsTask2:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn:
'Fn::GetAtt':
Expand All @@ -506,7 +506,7 @@ Resources:
ecsTask3:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn:
Ref: TaskArn
Expand Down
6 changes: 3 additions & 3 deletions guard-examples/deployment-safety/ecs-taskdef-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
ecsTask:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn: 'aws:arn'
ExecutionRoleArn: 'aws:arn2'
Expand All @@ -56,7 +56,7 @@
ecsTask:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn:
'Fn::GetAtt': [iam, Arn]
Expand All @@ -73,7 +73,7 @@
ecsTask:
Type: 'AWS::ECS::TaskDefinition'
Metadata:
SharedExectionRole: allowed
SharedExecutionRole: allowed
Properties:
TaskRoleArn:
'Fn::GetAtt': [iamRole, Arn]
Expand Down
4 changes: 2 additions & 2 deletions guard-examples/deployment-safety/ecs-taskdef.guard
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ rule check_ecs_task_role_refs_are_shared
when all_ecs_tasks_must_have_task_end_execution_roles
%task_role_shared !empty
{
%task_role_shared.Metadata.SharedExectionRole exists
%task_role_shared.Metadata.SharedExecutionRole exists
}

#
Expand All @@ -114,5 +114,5 @@ rule check_ecs_execution_role_refs_are_shared
when all_ecs_tasks_must_have_task_end_execution_roles
%execution_role_shared !empty
{
%execution_role_shared.Metadata.SharedExectionRole exists
%execution_role_shared.Metadata.SharedExecutionRole exists
}
4 changes: 2 additions & 2 deletions guard-examples/encryption/dynamodb-table-sse.guard
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ rule dynamo_db_sse_on when %ddb !empty
assert_all_resources_have_non_empty_tags
{
#
# Enusre ALL DynamoDB Tables have encryption at rest turned on
# Ensure ALL DynamoDB Tables have encryption at rest turned on
#
%ddb.Properties.SSESpecification.SSEEnabled == true
}
Expand Down Expand Up @@ -56,7 +56,7 @@ rule dynamo_db_sse_on_for_prod_only when dynamo_db_sse_on

#
# From the set of DynamoDB Tables that had SSE on (dependent rule dynamo_db_sse_on),
# check the ones that are targetted for PRODuction based on
# check the ones that are targeted for PRODuction based on
# Key containing /PROD/ and Value starting with /^App/
#
let only_prod_ddb = %ddb[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ rule prevent_outbound_access_to_any_ip when %sg_resources !empty {

when %egress !empty {
%egress.CidrIp != '0.0.0.0/0' <<IPv4 address can not be 0.0.0.0/0>> or
%egress.CidrIpv6 != '::/0' <<IPv6 addres cannot be ::/0>>
%egress.CidrIpv6 != '::/0' <<IPv6 address cannot be ::/0>>
}
}

Expand All @@ -23,7 +23,7 @@ rule prevent_inbound_access_to_any_ip when %sg_resources !empty {

when %ingress !empty {
%ingress.CidrIp != '0.0.0.0/0' <<IPv4 address can not be 0.0.0.0/0>> or
%ingress.CidrIpv6 != '::/0' <<IPv6 addres cannot be ::/0>>
%ingress.CidrIpv6 != '::/0' <<IPv6 address cannot be ::/0>>
}
}

2 changes: 1 addition & 1 deletion guard-lambda/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ AWS CLI version may have this configuration set to the required value.
aws lambda invoke \
--function-name $LAMBDA_FUNCTION_NAME \
--cli-binary-format raw-in-base64-out \
--payload '{"data":"{\"Resources\":{\"NewVolume\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":500,\"Encrypted\":true,\"AvailabilityZone\":\"us-west-2b\"}},\"NewVolume2\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":50,\"Encrypted\":true,\"AvailabilityZone\":\"us-west-2c\"}}}}","rules":["let ec2_volumes = Resources.*[ Type == /EC2::Volume/ ]\nrule EC2_ENCRYPTION_BY_DEFAULT when %ec2_volumes !empty {\n %ec2_volumes.Properties.Encrypted == true \n <<\n Violation: All EBS Volumes should be encryped \n Fix: Set Encrypted property to true\n >>\n}"],"verbose":false}' \
--payload '{"data":"{\"Resources\":{\"NewVolume\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":500,\"Encrypted\":true,\"AvailabilityZone\":\"us-west-2b\"}},\"NewVolume2\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":50,\"Encrypted\":true,\"AvailabilityZone\":\"us-west-2c\"}}}}","rules":["let ec2_volumes = Resources.*[ Type == /EC2::Volume/ ]\nrule EC2_ENCRYPTION_BY_DEFAULT when %ec2_volumes !empty {\n %ec2_volumes.Properties.Encrypted == true \n <<\n Violation: All EBS Volumes should be encrypted \n Fix: Set Encrypted property to true\n >>\n}"],"verbose":false}' \
output.json
```
Expand Down
2 changes: 1 addition & 1 deletion guard-lambda/tests/lambda-handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ mod tests {
use lambda_runtime::Context;

const NON_COMPLIANT_DATA: &str = "{\"Resources\":{\"NewVolume\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":500,\"Encrypted\":false,\"AvailabilityZone\":\"us-west-2b\"}},\"NewVolume2\":{\"Type\":\"AWS::EC2::Volume\",\"Properties\":{\"Size\":50,\"Encrypted\":true,\"AvailabilityZone\":\"us-west-2c\"}}}}";
const RULE: &str = "let ec2_volumes = Resources.*[ Type == /EC2::Volume/ ]\nrule EC2_ENCRYPTION_BY_DEFAULT when %ec2_volumes !empty {\n %ec2_volumes.Properties.Encrypted == true \n <<\n Violation: All EBS Volumes should be encryped \n Fix: Set Encrypted property to true\n >>\n}";
const RULE: &str = "let ec2_volumes = Resources.*[ Type == /EC2::Volume/ ]\nrule EC2_ENCRYPTION_BY_DEFAULT when %ec2_volumes !empty {\n %ec2_volumes.Properties.Encrypted == true \n <<\n Violation: All EBS Volumes should be encrypted \n Fix: Set Encrypted property to true\n >>\n}";
const FAILURE_MESSAGE: &str = "Failed to handle event";

#[tokio::test]
Expand Down
10 changes: 5 additions & 5 deletions guard/assets/cfn-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -304,25 +304,25 @@
"Action" : "lambda:InvokeFunction",
"FunctionName" : {
"Fn::GetAtt" : [
"EnableShapshot",
"EnableSnapshot",
"Arn"
]
},
"Principal" : "*"
}
},
"EnableShapshotTrigger" : {
"EnableSnapshotTrigger" : {
"Condition" : "EnableBackups",
"DependsOn" : [
"EnableShapshot",
"EnableSnapshot",
"LambdaExecutePermission",
"RedisReplicationGroup"
],
"Type" : "Custom::Region",
"Properties" : {
"ServiceToken" : {
"Fn::GetAtt" : [
"EnableShapshot",
"EnableSnapshot",
"Arn"
]
},
Expand All @@ -337,7 +337,7 @@
}
}
},
"EnableShapshot" : {
"EnableSnapshot" : {
"Condition" : "EnableBackups",
"Type" : "AWS::Lambda::Function",
"DeletionPolicy" : "Delete",
Expand Down
4 changes: 2 additions & 2 deletions guard/src/commands/reporters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
rules::{
self,
eval::eval_rules_file,
eval_context::{root_scope, simplifed_json_from_root, Messages},
eval_context::{root_scope, simplified_json_from_root, Messages},
exprs::RulesFile,
Status,
},
Expand Down Expand Up @@ -114,7 +114,7 @@ fn get_test_case<'rule>(
let root_record = root_scope.reset_recorder().extract();
let time = now.elapsed().as_millis();

let tc = match simplifed_json_from_root(&root_record) {
let tc = match simplified_json_from_root(&root_record) {
Ok(report) => match status {
Status::FAIL => {
let status = report.not_compliant.iter().fold(
Expand Down
4 changes: 2 additions & 2 deletions guard/src/commands/reporters/validate/cfn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
display::ValueOnlyDisplay,
errors::InternalError::UnresolvedKeyForReporter,
eval_context::{
simplifed_json_from_root, BinaryComparison, ClauseReport, EventRecord, FileReport,
simplified_json_from_root, BinaryComparison, ClauseReport, EventRecord, FileReport,
InComparison, RuleReport, UnaryComparison,
},
path_value::{
Expand Down Expand Up @@ -81,7 +81,7 @@ impl<'reporter> Reporter for CfnAware<'reporter> {
let root = data.root().unwrap();

if data.at("/Resources", root).is_ok() {
let failure_report = simplifed_json_from_root(root_record)?;
let failure_report = simplified_json_from_root(root_record)?;
match output_type {
OutputFormatType::YAML => serde_yaml::to_writer(write, &failure_report)?,
OutputFormatType::JSON => serde_json::to_writer_pretty(write, &failure_report)?,
Expand Down
4 changes: 2 additions & 2 deletions guard/src/commands/reporters/validate/generic_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::rules::{EvaluationType, Status};

use super::common::*;
use super::summary_table::SummaryType;
use crate::rules::eval_context::{simplifed_json_from_root, EventRecord};
use crate::rules::eval_context::{simplified_json_from_root, EventRecord};
use crate::rules::path_value::traversal::Traversal;
use crate::rules::values::CmpOperator;

Expand Down Expand Up @@ -125,7 +125,7 @@ impl Reporter for GenericSummary {
_data: &Traversal<'value>,
output_type: OutputFormatType,
) -> crate::rules::Result<()> {
let failure_repord = simplifed_json_from_root(root_record)?;
let failure_repord = simplified_json_from_root(root_record)?;

match output_type {
OutputFormatType::JSON => serde_json::to_writer_pretty(writer, &failure_repord)?,
Expand Down
Loading

0 comments on commit 2b5d57d

Please sign in to comment.