Skip to content

[FrameworkBundle] secrets:decrypt-to-local shows hard error if invalid secret is found #42038

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.

Sign up for GitHub

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

Jump to bottom
Closed
wouterj opened this issue Jul 9, 2021 · 1 comment
Closed

[FrameworkBundle] secrets:decrypt-to-local shows hard error if invalid secret is found #42038

wouterj opened this issue Jul 9, 2021 · 1 comment
Labels
Bug FrameworkBundle Status: Reviewed

Comments

@wouterj
Copy link
Member

wouterj commented Jul 9, 2021

Symfony version(s) affected: 4.4

Description
If I wrongly remove a secret (i.e. I don't use secrets:remove and only remove the file and don't update the list), you get a hard error in secrets:decrypt-to-local:

$ symfony console secrets:decrypt-to-local --force -vv

 // 2 secrets found in the vault.

[critical] Error thrown while running command "secrets:decrypt-to-local --force -vv". Message: "Symfony\Component\Console\Formatter\OutputFormatter::escape():
Argument #1 ($text) must be of type string, null given, called in /home/wouter/projects/local/symfony/secrets/vendor/symfony/console/Style/SymfonyStyle.php on
line 476"


In OutputFormatter.php line 41:

  [TypeError]
  Symfony\Component\Console\Formatter\OutputFormatter::escape(): Argument #1 ($text) must be of type string, null given, called in /home/wouter/projects/loc
  al/symfony/secrets/vendor/symfony/console/Style/SymfonyStyle.php on line 476

How to reproduce
I created an app in this state: https://github.com/wouterj/sf-reproducer/tree/secrets-bad-remove

Run the above command and you'll get the hard error.

Possible Solution
Vault::list() reveals all keys using Vault::reveal(). This method resets the last error message each call. The secrets:decrypt-to-local command expects there to be a last error message if the value is null, but this is not the case if a valid secrets is available in the vault after the bad one.

We should either fail in Vault::list() as soon as a bad secret is found, or allow lastMessage to be an array of messages.

Additional context
n/a
@wouterj wouterj added the Bug label Jul 9, 2021
@gnito-org
Copy link
Contributor

Status: Reviewed

I was able to reproduce this bug using the supplied code. The problem also exists in Symfony 5.3.

However, it produces the error message Symfony\Component\Console\Formatter\OutputFormatter::escape(): Argument #1 ($text) must be of type string, null given, called in /xxx/symfony-contribution/sf-reproducer/vendor/symfony/console/Style/SymfonyStyle.php on line 476 on any missing secret except the bottom one in config/secrets/dev/dev.list.php.

If there is a secret that exists above the non-existent bottom one in the array, then the error message is [ERROR] Secret "[BOTTOM ONE]" not found in "config/secrets/dev/".

For example:

Given config/secrets/dev/dev.list.php contains:

return [
    'A1THING' => null,
    'ANOTHERTHING' => null,
    'OTHERTHING' => null,
    'ZTHING' => null,
];

If any combination of A1THING, ANOTHERTHING, and OTHERTHING does not exist and ZTHING does exist, then the code crashes with the ($text) must be of type string, null given error message.

If any combination of A1THING, ANOTHERTHING, and OTHERTHING does exist and ZTHING does not exist, or none of the four exist, then you get the [ERROR] Secret "ZTHING" not found in "config/secrets/dev/" error message. Note that it always only complains about the bottom one being missing.

@94noni 94noni mentioned this issue Sep 22, 2021
Merged
@xabbuh xabbuh mentioned this issue Jul 22, 2024
Merged
xabbuh added a commit to xabbuh/symfony that referenced this issue Jul 22, 2024
@xabbuh
terminate with non-zero exit code when a secret could not be read
1d1ab26 
The fix for symfony#42038 did a bit too much. Not only did it fix the PHP error that
wasn't caught before, but also changed the exit code of the command. With this
change the PHP error will still be prevented, but the command will terminate
with a non-zero exit code to indicate the failure that occurred while reading
the stored secrets.
chalasr added a commit that referenced this issue Jul 22, 2024
@chalasr
feature #57797 [FrameworkBundle]  terminate with non-zero exit code w…
450113c 
…hen a secret could not be read (xabbuh)

This PR was merged into the 7.2 branch.

Discussion
----------

[FrameworkBundle]  terminate with non-zero exit code when a secret could not be read

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        |
| License       | MIT

The fix for #42038 did a bit too much. Not only did it fix the PHP error that
wasn't caught before, but also changed the exit code of the command. With this
change the PHP error will still be prevented, but the command will terminate
with a non-zero exit code to indicate the failure that occurred while reading
the stored secrets.

Commits
-------

1d1ab26 terminate with non-zero exit code when a secret could not be read
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@wouterj @carsonbot @gnito-org