Protecting against malicious PRs

[mykter]

I think this is a very neat idea, great job on coming up with it!

A couple of attacks come to mind, this is one of them.

Could a malicious PR obtain credentials? An attacker:

1. Forks the repo
2. Changes the workflow to send the creds to themself
3. Creates a PR.
4. The action runs, the lambda generates creds, and the subverted job sends them on to the attacker.

Or is that what this check is protecting against? The comment mentions forks, I'm not familiar enough with the github api to know if this is protecting against PRs running in the genuine repo but sourced from a fork.

If this is already protected against (and the lambda won't return creds for PRs sourced from forks), then I think it would be worth mentioning in the README.

[kapilt]

this still uses github secrets, which are only executed against trunk/master/main not against prs.

[mykter]

True, it wouldn't work directly in the example workflow given, however the two values stored in github secrets are the endpoint URL and account ID. Generally speaking you can't assume those two values are secrets in the same way that credentials are secret - they can get leaked in various ways, so our hypothetical attacker could replace the use of the 'secrets' with their actual static values.

Indeed, the goal of this tool as I understand it is to avoid the need to have secrets in GitHub - if we're doing that then you may as well do the simple option of storing AWS creds in GitHub.

[aidansteele]

@mykter My apologies for not responding significantly sooner. I must have misconfigured my GitHub notification settings as I don't recall getting an email about this.

With regards to this code snippet you referenced:

actions2aws/api/api.go

Lines 88 to 91 in bcc43f9

	if run.Repository.ID != run.HeadRepository.ID && run.Repository.ID != 0 {
	// this is a fork, no credentials for you
	return nil, errors.New("no credentials for a fork")
	}

That code will return an error if the CI run is for a commit in a fork of the repo. If the commit is pushed to the origin, it will run - but other repos (forks) will fail. My implicit assumption is that code committed to any branch in the origin is trustworthy, but I should make this assumption explicit as you rightly point out.

Regarding the usage of GitHub secrets: again you are correct in that these aren't highly sensitive secrets. They're more there for protection of information from casual snooping. If they were for some reason printed to log files it shouldn't be a big deal.

[mykter]

That makes sense, thanks.