Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CICD Improvements #400

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

CICD Improvements #400

wants to merge 6 commits into from

Conversation

cat5inthecradle
Copy link
Contributor

No description provided.

@cat5inthecradle
Copy link
Contributor Author

I've manually deployed the dependencies stack to create the new role, but before testing the rest, I'd like to get a review.

Comment on lines +92 to +98
# - PolicyName: CloudformationPackage
# PolicyDocument:
# Statement:
# - Effect: Allow
# Action:
# - TBD
# Resource: TBD
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need this permission

Copy link

@unlox775-code-dot-org unlox775-code-dot-org left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. I spent some time digging into how we were using IAM-DevPermissions as a Permission boundary. The part that didn't sit well with me was using a automated system that in any way referenced the role used by human users. As a permission boundary, I don't think it harms the security here. And everything else looks good, so I'm happing shipping this PR as-is

In future I am thinking I would prefer to make sandbox-style allow list permission boundaries. The main thing I would propose, at least to start the discussion, is to move away from block-list style rulesets, that is using negative rule logic, Deny, or NotAction or StringNotEquals.

Though not specifically for this PR, I put together a POC to test some of my thoughts on blocklist style access. See here: https://github.com/unlox775/security_poc/tree/main/passrole_star

don't remember the context for this...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants