-
Notifications
You must be signed in to change notification settings - Fork 52
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
Bug: Output of harden-runner insight should use commit hash instead #61
Comments
Thanks @jauderho for using harden-runner! Good feedback. I will address it ASAP. I observed you are setting the allowed-endpoints, but the egress-policy is set to audit. As a result the traffic is not restricted. The default value for egress-policy is block. So you can either remove egress-policy and just keep the allowed-endpoints or set egress-policy to block. I will address this in the insights page as well and set egress-policy to block explicitly. I will be happy to fix your workflows for you. Please let me know. |
@varunsh-coder , I was leaving it to |
@varunsh-coder while you are at it, if you can make the copy block on the insights page be the entire workflow (or at least make than an option), it will make it easier to do a complete copy and replace. |
@jauderho Great idea! I will make it same experience as the home page https://app.stepsecurity.io, where there is an editor and one can just copy and paste it without having to fix indentation etc. Do let me know if you have more ideas to improve user experience or any other feedback. Thanks a lot! |
I do have one nit but it's mostly me being OCD. If I check the Also, do the insight links expire after a while? For example, https://app.stepsecurity.io/github/jauderho/psfiles/actions/runs/1731266664 |
Good feedback! I will address it. Please keep the feedback coming!
No, they do not expire. Did you want them to expire? |
Might be nice to have them expire in a few days (and an option to retain if needed). I’m just wondering about the use case where a private repo might accidentally leak endpoints if this is integrated in the workflow. |
Also, the way things are set up means that we have to take two passes, once to audit and once to block. I’m trying to think of a safe way to do this in one pass. I suppose one could have an action that modifies things after the fact but that would mean that the action needs write access and self modifying code could have unexpected outcomes. Although, I think your tool appears to be able to have some knowledge of various actions so maybe that can be included as part of the suggested block of yaml. For instance, the CodeQL action is almost always going to be talking to the same set of hosts for everyone so it would make sense to just return github.com etc. as part of the allow list. Does that make sense? |
Hi @jauderho |
Yes, it would be ideal if one did not have to change the workflow while going from
That knowledge base is to give users some information about why a certain domain is called. But restricting based on that will not work for |
Yeah, not allowing for private repos without an explicit opt-in would be good. For example, I now have a GHA template repo which I plan to use to copy to new repos. Not allowing private repos will prevent inadvertent exposure. |
@jauderho since you have a repo related to docker images, and you run security tools on those images, I wanted to ask you about this feature I am thinking about - automatic SBOM (software bill of materials)/ provenance generation. As you know This way developers will not need to think about SBOM generation, it will just happen as part of the build. What do you think about this? Thanks! |
It's on my todo list to figure out how to implement cosign and SBOMs for containers. I just figured it out for Go binaries (see bl3auto repo) but have not had cycles to work on this yet. Hopefully in the next few weeks... |
@jauderho I have fixed the commit hash issue that you reported. The For all your other suggestions, I have created separate issues to track them. I have improved the copy-paste experience, but it will take me more time to have the full workflow show up there. I will get it done soon though. Thanks again for the feedback! I have added a Slack channel link in the readme, in case you want to join. Would love to have more discussions. I will go ahead and close this issue. Thanks! |
I see the fix. However, to match exactly as the other page, there should be a
|
@jauderho sorry, missed that. re-opening issue. will add it soon. |
One observation: My While this is better, I've noticed that this changes the output of the insights URL from say Looks like the username is masked now. Not a showstopper but just to point out that the URL will not work. You might want to have a more verbose error message for folks that hit the |
Thanks for reporting this! This is definitely going to cause problems for some users as they will not know what is wrong with the URL. I will create another issue for it. |
Noticed that My understanding is that node 12 support was EOL (normal not security) a while ago. See https://endoflife.date/nodejs Should this be bumped to say node 16? |
Thanks a lot for feedback! I will create an issue to address. |
If api.snapcraft.io is blocked for Or should there be a block-hard-fail option type? Also, the corresponding output |
Yes, I think I need user feedback on this. Let me create an issue to investigate |
Maybe it should be So you end up with just:
It's unclear to me how much value is added by having a |
So I've run into a potential issue around how the allowed-endpoints are defined. If you look at my workflow for ansible ( Now the dependencies and hence the allowed-endpoints are different for ubuntu vs. alpine as some python wheels have to be manually built for alpine dependencies. In order to make this run successfully without having to break out the matrix into different jobs, I would have to end up using whatever the more permissive list would be. Not a showstopper but might catch some folks down the line. Example: https://app.stepsecurity.io/github/jauderho/dockerfiles/actions/runs/1742888941 |
Interesting case. Yes, I think the union of all endpoints for the matrix would need to be used in the workflow file. When the experience of showing the entire workflow in the editor in the insights page gets done, getting a list of all the endpoints across jobs in a matrix should get easier. |
The main difference is in terms of where the |
Where is this backend that you are referring to? I think that's where I'm missing understanding of. |
Backend is the API and data store that stores the correlated outbound traffic. Now, the idea is that instead of adding the The downside is that the |
@jauderho I have fixed the UI bug. Can you please verify? The editor should show
|
You may want to track signing with Docker here : docker/roadmap#269 |
This appears to work. Just tested on one of my actions. |
BUG: https://app.stepsecurity.io/github/jauderho/dockerfiles/actions/runs/1747044469 does not appear to return any output. This is tied to a pretty large job with 150+ subtasks (51 simultaneous). Workflow file is here: https://app.stepsecurity.io/github/jauderho/dockerfiles/actions/runs/1747044469 It is unclear to me if the large number of jobs is causing an issue somewhere. |
I had created a bug to investigate this earlier - #68. |
Thanks! I will go ahead and close this bug. I will enable discussions on this repo. |
So.... I think I have keyless cosign and SBOM generation working for a container image. There are a bunch of moving parts so this may yet change. Workflow is here: https://github.com/jauderho/dockerfiles/blob/main/.github/workflows/age.yml Output is here: https://github.com/jauderho/dockerfiles/actions/runs/1755633128 EDIT: I have not yet figured out how to use cosign to sign the scan output but it does seem possible. See https://github.com/pvnovarese/oss-2021-sbom-complete-workflow-demo/blob/main/.github/workflows/build-and-publish.yaml |
Thanks @jauderho! I have created a discussion topic for SBOM and automatic signing. |
A somewhat related question for you. One of the OpenSSF Scorecard recommendations is to switch from using Docker image tag to digests. Given that you have an Action to covert from versions to commit hashes for GHA, do you know of or have any plans to create a similar page to https://app.stepsecurity.io/ that will allow for a cut/paste conversion for Docker tags? |
@jauderho I was thinking of the exact same thing. I will add it. Thanks for the idea! |
One item that's unclear to me is we should include the version number while using the digest for point to an image. See jauderho/dockerfiles#146 |
Yes, I think we should. |
I just testing and implementing harden-runner after starting with the scorecard action. Repo is here: https://github.com/jauderho/psfiles
So if the push is for actions to use commit hashes instead of version tags, the output page of the insights should utilize the commit hash instead.
For example, https://app.stepsecurity.io/github/jauderho/psfiles/actions/runs/1731266664
Recommendation is for
Instead, it really should be (and changing as necessary as the action gets updated)
Similarly, the README.md for this repo should indicate/recommend the use of hashes instead.
The text was updated successfully, but these errors were encountered: