CDK-nag actions from external forks do not have write permissions #766
Labels
Milestone
Comments
dlpzx
added
type: bug
|
Hi @dlpzx This is marked as high priority and in progress, if you are working on it id like to add it to v2.1.0 milestone to track it. |
dlpzx
added a commit
that referenced
this issue
Sep 19, 2023
### Feature or Bugfix - Bugfix ### Detail See issue #766 Alternatives considered: 1. use [ok-to-test](https://github.com/imjohnbo/ok-to-test) --> requires authentication, preferred is GitHub App (not possible for data.all), other options: personal access token or OAuth app token 2. run cdk-nag action only on minor release branches (v2m2m0 to main) branch 3. run cdk-nag on `pull_request_target` after the PR is merged 4. run cdk-nag on schedule 5. Use other than OIDC but I think the issue could still be there as it has to due with permissions on the repo 6. Avoid the need for credentials This last one is the cleanest and safest. We need to mock the context of the cdk app either: - passing context as part of the CLI command `cdk synth --context key=value` --> not possible as we need to pass more complex params - creating a json object in the CLI --> cumbersome - CHOSEN: pass the context directly in the declaration of the `App` In addition other changes had to be made: - remove need for SSM calls in app.py if GithubActions are running - try/except in getting the S3 prefixes⚠️ STILL NEEDS TESTING ### Relates #766 ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
|
Completed in #767 |
anmolsgandhi
removed
the
status: in-progress
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
As happens in this PR, the cdk-nag action requires write permissions, but the maximum permissions allowed for a fork in a public repo are
readpermissions.alternatives
pull_request_targetafter the PR is mergedAdvice:
6 woudl solve the issue, so preferred if possible. 2 and 3 can also be added.
How to Reproduce
Open a PR from a fork.
Expected behavior
No response
Your project
No response
Screenshots
No response
OS
n/a
Python version
n/a
AWS data.all version
7.0.0
Additional context
No response
The text was updated successfully, but these errors were encountered: