From fff371ea59f8a4b60079a7c4d0b73b202a99324e Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 6 Jun 2026 09:19:42 -0700 Subject: [PATCH] Fix broken secrets-file restore on failed credential rotation restore_secrets_file copied the live secrets file onto the backup instead of copying the backup back onto the live secrets file. On a failed rotation the rollback therefore did nothing useful and also overwrote the only good backup with the (possibly broken) current secrets, leaving the server with unusable credentials and no way to recover them. Copy backup -> live secrets file so a failed rotation can actually roll back. Add a regression test; the existing tests stubbed this method out entirely, so the direction bug was never exercised. Signed-off-by: Tim Smith --- .../plugins/rotate_credentials.rb | 2 +- .../spec/rotate_credentials_spec.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/chef-server-ctl/plugins/rotate_credentials.rb b/src/chef-server-ctl/plugins/rotate_credentials.rb index 2dfdd1180a..0448fdbfe2 100644 --- a/src/chef-server-ctl/plugins/rotate_credentials.rb +++ b/src/chef-server-ctl/plugins/rotate_credentials.rb @@ -221,7 +221,7 @@ def backup_secrets_file(backup_file = nil) def restore_secrets_file(backup_file) log("Restoring #{backup_file} to #{secrets_file_path}...") - FileUtils.cp(secrets_file_path, backup_file) + FileUtils.cp(backup_file, secrets_file_path) end def secrets_file_path diff --git a/src/chef-server-ctl/spec/rotate_credentials_spec.rb b/src/chef-server-ctl/spec/rotate_credentials_spec.rb index 8dc6c735b8..901d19e853 100644 --- a/src/chef-server-ctl/spec/rotate_credentials_spec.rb +++ b/src/chef-server-ctl/spec/rotate_credentials_spec.rb @@ -169,6 +169,25 @@ def credentials end end + context "restore_secrets_file" do + it "copies the backup back over the live secrets file" do + allow(subject.ctl).to receive(:restore_secrets_file).and_call_original + allow(subject.ctl) + .to receive(:secrets_file_path) + .and_return("/etc/opscode/private-chef-secrets.json") + + # The restore must copy backup -> live secrets file. Copying in the + # other direction would overwrite the only good backup with the + # (possibly broken) current secrets, making a failed rotation + # unrecoverable. + expect(FileUtils) + .to receive(:cp) + .with("/tmp/backup.json", "/etc/opscode/private-chef-secrets.json") + + subject.ctl.send(:restore_secrets_file, "/tmp/backup.json") + end + end + context "require_credential_rotation_pre_hook" do let(:credential_rotation_required_file) do "/tmp/var/opt/opscode/credential_rotation_required"