Skip to content

Fix: keep ssh and scp targets and redact common password forms - #196

Open
aayank13 wants to merge 1 commit into
mainfrom
fix/redact-ssh-targets-and-passwords
Open

aayank13 wants to merge 1 commit into
mainfrom
fix/redact-ssh-targets-and-passwords

Conversation

@aayank13

Copy link
Copy Markdown
Owner

Fixes #165

The email rule treated every user@host.tld as an email, so ssh deploy@staging.example.com became ssh [REDACTED:email] and scp build.tar ubuntu@ec2-1-2-3-4.compute.amazonaws.com:/tmp lost 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/shop and then npm test was 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:6379 was learned, and the password was stored in plain text in the procedure's steps.

A user@host is now kept when it is followed by :/ or :~ (a remote path), and when it is a login target of scp, rsync, sftp, sshfs and ssh-copy-id, or the destination or -J jump host of ssh, autossh and mosh. The program has to start the command, optionally after env assignments or a wrapper like sudo, timeout or sshpass, and with no quoted word before it. So git commit -m 'use ssh ada@example.com' stays redacted. For ssh only the words before the remote command count, so ssh deploy@host mail ada@example.com still 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, so postgres://app:p@ss@db no longer leaks ss. 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 plain token or key word separated by a space, as in the key <hex>. Hash words like sha256 and flags like gpg --recv-key are still left alone. New rules cover docker login -p (and podman, buildah, skopeo, nerdctl, helm and oras), redis-cli -a (and valkey and keydb), the password argument of htpasswd -b, and MYSQL_PWD-style names that end in pwd. The Authorization header rule now also handles JSON and JS forms like {"Authorization": "Bearer ..."} and the Bot, Negotiate and ApiKey schemes. It also redacts a header value with no scheme, but only when the value mixes letters and digits, so code like authorization: Optional[str] is left alone. Separately, the curl -u rule no longer treats an earlier email marker as a username, which broke idempotency once curl -u ada@example.com:pass was redacted twice.

These were already handled and are now covered by tests: --password=..., mysql -p<pass>, PASSWORD=... and PGPASSWORD=... assignments, https://user:pass@host URLs and Authorization: Bearer|Basic|token headers. Left out on purpose: git push deploy@host:repo.git and other non-ssh targets without a :/ or :~ path stay redacted. A target like that can't be told apart from email: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 stored redis://:hunter2@localhost:6379 in 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 and curl -u that 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.

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.

Email redaction breaks ssh/scp targets while common password forms leak

1 participant