Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #165
The email rule treated every
user@host.tldas an email, sossh deploy@staging.example.combecamessh [REDACTED:email]andscp build.tar ubuntu@ec2-1-2-3-4.compute.amazonaws.com:/tmplost its target. Since a step with a redaction marker is rejected, deploy procedures were never learned. Ingesting a session that edits a file, runs./scripts/deploy.sh deploy@staging.example.com:/srv/shopand thennpm testwas rejected with "a step depends on a redacted secret". The other side was worse: several common password forms passed through untouched. The same session shape running./scripts/seed.sh redis://:hunter2@localhost:6379was learned, and the password was stored in plain text in the procedure's steps.A
user@hostis now kept when it is followed by:/or:~(a remote path), and when it is a login target ofscp,rsync,sftp,sshfsandssh-copy-id, or the destination or-Jjump host ofssh,autosshandmosh. The program has to start the command, optionally after env assignments or a wrapper likesudo,timeoutorsshpass, and with no quoted word before it. Sogit commit -m 'use ssh ada@example.com'stays redacted. Forsshonly the words before the remote command count, sossh deploy@host mail ada@example.comstill redacts the address in the remote command. To check this, a rule's harmless check now also gets the text being redacted, not just the match.On the leak side, the URL password rule now accepts an empty user, as in
redis://:pass@host, and takes the password up to the last@in the authority, sopostgres://app:p@ss@dbno longer leaksss. A 32+ character hex user in a URL (https://<hex>@github.com/...) is redacted as a token. Previously it was only caught because it happened to look like an email. A long hex value is now also caught after a plaintokenorkeyword separated by a space, as inthe key <hex>. Hash words likesha256and flags likegpg --recv-keyare still left alone. New rules coverdocker login -p(and podman, buildah, skopeo, nerdctl, helm and oras),redis-cli -a(and valkey and keydb), the password argument ofhtpasswd -b, andMYSQL_PWD-style names that end inpwd. The Authorization header rule now also handles JSON and JS forms like{"Authorization": "Bearer ..."}and theBot,NegotiateandApiKeyschemes. It also redacts a header value with no scheme, but only when the value mixes letters and digits, so code likeauthorization: Optional[str]is left alone. Separately, the curl-urule no longer treats an earlier email marker as a username, which broke idempotency oncecurl -u ada@example.com:passwas redacted twice.These were already handled and are now covered by tests:
--password=...,mysql -p<pass>,PASSWORD=...andPGPASSWORD=...assignments,https://user:pass@hostURLs andAuthorization: Bearer|Basic|tokenheaders. Left out on purpose:git push deploy@host:repo.gitand other non-ssh targets without a:/or:~path stay redacted. A target like that can't be told apart fromemail:password, so I'd rather reject the step than leak an address. Likewise, a scheme-less Authorization value made only of letters is not redacted, to keep code and prose usable.Tested end to end with the built binary and a scratch
TRODDEN_HOME, ingesting the two sessions above. Before the fix, the deploy session was rejected and the seed session storedredis://:hunter2@localhost:6379in the learned procedure. After the fix, the deploy procedure is learned with its target intact, and the seed session is rejected because of the redacted password. The redaction corpus gains the ssh, scp, rsync, sftp and ssh-copy-id targets that must be kept, every password form above that must be redacted, emails inside ssh remote commands, commit messages andcurl -uthat must stay redacted, and exact outputs for the URL, docker, htpasswd and ssh cases. Four of the tests fail on the old code. The idempotency tests and the quickcheck property still pass.