sops icon indicating copy to clipboard operation
sops copied to clipboard

3.7.2 - error loading config: no matching creation rules found

Open rayterrill opened this issue 2 years ago • 21 comments

Did 3.7.2 introduce some breaking changes/functionality?

Looks like when we upgrade to 3.7.2, we're unable to decrypt kms secrets on mac amd64. Downgrading to 3.7.1 works fine.

Users are stumbling into this as brew is upgrading their tools to 3.7.2.

Error is:

me@C02FR4M4MD6R dev-env % helm secrets dec ../secrets.yaml
[helm-secrets] Decrypting ../secrets.yaml
error loading config: no matching creation rules found
[helm-secrets] Error while decrypting file: ../secrets.yaml
Error: plugin "secrets" exited with error

rayterrill avatar Mar 17 '22 16:03 rayterrill

For now we're reverting users running into the problem:

#!/bin/bash

curl -L https://github.com/mozilla/sops/releases/download/v3.7.1/sops-v3.7.1.darwin -o sops --silent
chmod +x sops
sudo mv sops /usr/local/bin/sops

rayterrill avatar Mar 17 '22 16:03 rayterrill

I'm running into this as well on MacOS 11.6.4.

danmanners avatar Mar 17 '22 19:03 danmanners

Yes, it's because of this: https://github.com/mozilla/sops/pull/853

You need to change the path regex to be the relative path of the config file. For example:

  • The file you want to match is in ~/a/b/encrypted/secret.file
  • Your .sops.yaml is in ~/a/.sops.yaml Then you need to have your creation_rules.path or creation_rules.path_regex to be b/encrypted/secret.file instead of the full path ~/a/b/encrypted/secret.file. I don't know what is the reason of this change because I got that same error too and after modifying my config file it's working again.

budimanjojo avatar Mar 18 '22 15:03 budimanjojo

Yes, it's because of this: #853

You need to change the path regex to be the relative path of the config file. For example:

* The file you want to match is in `~/a/b/encrypted/secret.file`

* Your `.sops.yaml` is in `~/a/.sops.yaml`
  Then you need to have your `creation_rules.path` or `creation_rules.path_regex` to be `b/encrypted/secret.file` instead of the full path `~/a/b/encrypted/secret.file`. I don't know what is the reason of this change because I got that same error too and after modifying my config file it's working again.

That is a breaking change... I have also reverted to 3.7.1 in order to be able to work, and have instructed my team to avoid future updates of SOPS until we confirm this is fixed. Since this breaking change is intentional, it probably should not be a patch release... We have many people using SOPS across many repositories, some of which are customer-owned, and it is highly irregular to insist that everyone upgrade their configuration and update all of the repositories for a non-major version change.

chrischildresssg avatar Mar 21 '22 13:03 chrischildresssg

Yes, it's because of this: #853

You need to change the path regex to be the relative path of the config file. For example:

  • The file you want to match is in ~/a/b/encrypted/secret.file
  • Your .sops.yaml is in ~/a/.sops.yaml Then you need to have your creation_rules.path or creation_rules.path_regex to be b/encrypted/secret.file instead of the full path ~/a/b/encrypted/secret.file. I don't know what is the reason of this change because I got that same error too and after modifying my config file it's working again.

This does fix it. I'm definitely with @chrischildresssg; I'm astounded that 3.7.2 (a minor change according to semantic versioning) broke things. Easy enough to update a handful of small repos, but holy hell.

I still think I'll take this over the project being abandonware though...😉

danmanners avatar Mar 21 '22 17:03 danmanners

It should also be noted that ./b/encrypted/secret.file (a very common syntax for relative paths) ALSO exhibits the failing behavior.

Manually reverting the version to 3.7.1 per @rayterrill 's comment resolved the behavior. Appreciate the workaround.

blancharda avatar Mar 24 '22 20:03 blancharda

I believe that's because, as the name indicates, it's supposed to be a regex, which doesn't support shellscript-related expansion shortcuts.

As it introduces breakage for users, I can send a PR supporting tilde expansion (~/), but what about other shortcuts, like ./ or ../? That would conflict with regex symbols.

paulolieuthier avatar Mar 24 '22 22:03 paulolieuthier

It looks like we're breaking on "./". I'm having a hard time understanding how this worked before - was this not being eval'd as a regex previously - and now it is?

rayterrill avatar Mar 24 '22 22:03 rayterrill

Looking at the PR diff I tossed together a toy example in go playground to compare the "old" method (no path trimming) and the new method (trim to a relative path).

It definitely looks like Go's regexp package fires a match for regex string ./folder/secret.file against a full path like /home/user/some-app/folder/secret.file - but not against a path trimmed relative path like folder/secret.file.

blancharda avatar Mar 24 '22 23:03 blancharda

I believe the leading '.' matches any character, meaning the actual matched text in the old method would have been the last letter of the folder and the rest of the match string. Something like p/folder/secret.file.

blancharda avatar Mar 25 '22 00:03 blancharda

To be clear - that's a bit of a fluke, and obviously not the intended regex match... 😂
But it was definitely functioning before the update. I doubt we're the only ones running into this edge case - let alone the broader implication of the change in general which makes assumptions about the path structure.

blancharda avatar Mar 25 '22 00:03 blancharda

I don't know much about coding, but looking at the commit, function parseCreationRuleForFile is taking filePath as parameter and inside the function the variable filePath, then it reconfigure filePath with this line: filePath = strings.TrimPrefix(filePath, configDir + string(filepath.Separator)) which is trimming away anything that match in the front including path separator. So if the "real" filePath was /a/b/c/dddd and your confPath is /a/b/c/.sops.yaml, that code will turn your filePath into just dddd, which is indeed the relative file path to the confPath. I think these lines https://github.com/mozilla/sops/blob/86f500de6102f5219e3fd0a25c718db01a7d39ed/config/config.go#L343-L346 should check both ./dddd or dddd instead of just dddd.

budimanjojo avatar Mar 25 '22 07:03 budimanjojo

Also need to add that I was wrong. I just looked at my path_regex and I'm using path_regex: ".*\/?filepath and not path_regex: ".\/filepath" which explains why mine work, because I added the ? :) Also I was wrong about creation_rules.path is also affected, it seems like this only affect creation_rules.path_regex, which is weird because it breaks consistency.

budimanjojo avatar Mar 25 '22 07:03 budimanjojo

The reason why I sent that PR is because parent directories with the same name as the intended subdirectory for the match were matched. Example:

Path regex: foo/* Sops file: /path/to/project/foo/.sops.yaml This should be included: /path/to/project/foo/foo/* This should not be included: /path/to/project/foo

That's why it made sense for me, and it was accepted for merge, to consider matching the regex to paths relative to the location of the sops configuration file.

It's weird to me to have sops alter a file in an upper level or disjoint branch of the file hierarchy. But I also understand the frustration of an unexpected breakage, which I didn't antecipate.

paulolieuthier avatar Mar 25 '22 10:03 paulolieuthier

@paulolieuthier yes you're right. But the problem now is ./path/to/project/foo/foo should also be included which it isn't right now. Maybe something like this?

if reg.MatchString(filePath) || reg.MatchString("./" + filePath) {

budimanjojo avatar Mar 25 '22 12:03 budimanjojo

But the problem now is ./path/to/project/foo/foo should also be included which it isn't right now.

Why is that? It doesn't make sense regex-wise. Maybe the project should change to support both regex and file path globbing. File path globbing is supported by golang:

https://pkg.go.dev/path/filepath#Glob https://pkg.go.dev/path/filepath#Match https://pkg.go.dev/path/filepath#Clean

But that wouldn't cover tilde expansion (~/), like in your original example, as that's in the scope of the command line shell.

paulolieuthier avatar Mar 25 '22 13:03 paulolieuthier

The reason why I sent that PR is because parent directories with the same name as the intended subdirectory for the match were matched. Example:

Path regex: foo/* Sops file: /path/to/project/foo/.sops.yaml This should be included: /path/to/project/foo/foo/* This should not be included: /path/to/project/foo

That's why it made sense for me, and it was accepted for merge, to consider matching the regex to paths relative to the location of the sops configuration file.

It's weird to me to have sops alter a file in an upper level or disjoint branch of the file hierarchy. But I also understand the frustration of an unexpected breakage, which I didn't antecipate.

Understood, and agreed. It does seem odd for a sops file to configure rules for files/folders above its current location in the file tree when thinking about a single project. Codifying it however makes an assumption about usage that I don't think is safe.. It's not too crazy to think that a common sops file might be used to manage multiple projects with a structure something like:

.
├── common_config
│   └── sops.yaml
├── proj_a
└── proj_b

If the sops file is expected to be at the top of all managed contexts, we should make sure that's clearly documented, and it might even make sense to remove the --config flag (which allows you to bypass the recursive search).

I also think it's fair to say that a leading ./ in the match string was incorrect regex and it doesn't make sense to preserve incorrect usage. I'll definitely be updating that on my end ASAP.

To your example, I know it's a simplified case - but isn't the real problem that the regex isn't specific enough? Wouldn't path regex foo/foo/* resolve the error case?

In any case, I think trimming the base path off the file location will still cause breaking changes for anyone who was specifying full paths in the match criteria which, before this change, was technically correct and functional (albeit somewhat odd).

blancharda avatar Mar 25 '22 13:03 blancharda

But the problem now is ./path/to/project/foo/foo should also be included which it isn't right now.

Why is that? It doesn't make sense regex-wise. Maybe the project should change to support both regex and file path globbing. File path globbing is supported by golang:

https://pkg.go.dev/path/filepath#Glob https://pkg.go.dev/path/filepath#Match https://pkg.go.dev/path/filepath#Clean

But that wouldn't cover tilde expansion (~/), like in your original example, as that's in the scope of the command line shell.

Supporting globbing sounds like a nice feature :) It might help simplify future usage.

blancharda avatar Mar 25 '22 13:03 blancharda

To your example, I know it's a simplified case - but isn't the real problem that the regex isn't specific enough? Wouldn't path regex foo/foo/* resolve the error case?

In my real case, I have the following structure:

~/code/company/repo-with-continuous-integration-config-for-all-projects
├─ .sops.yaml
├─ projectA/
├─ [...]
├─ company # repository with company name
├─ [...]
└─ projectZ/

And .sops.yaml:

- path_regex: projectA/.*
  [...]
[...]
- path_regex: company/.*
  [...]
[...]
- path_regex: projectZ/.*
  [...]

We have different keys for each project, but sops was using only the key for the project company because it matched with the prefix path ~/code/company.


It's not too crazy to think that a common sops file might be used to manage multiple projects with a structure something like:

.
├── common_config
│   └── sops.yaml
├── proj_a
└── proj_b

Fair enough. I can suggest two solutions for sops maintainers:

  • Support a path_glob configuration option as an alternative to path_regex. That's nice, but path_regex shouldn't be limited to only some use cases.
  • Support a way to define the project root, as many development tools do with .git, .hg, and the like. That way your regex expression would work relative to the project root even though the sops config file would be located elsewhere.

paulolieuthier avatar Mar 25 '22 14:03 paulolieuthier

But the problem now is ./path/to/project/foo/foo should also be included which it isn't right now.

Why is that? It doesn't make sense regex-wise. Maybe the project should change to support both regex and file path globbing. File path globbing is supported by golang:

https://pkg.go.dev/path/filepath#Glob https://pkg.go.dev/path/filepath#Match https://pkg.go.dev/path/filepath#Clean

But that wouldn't cover tilde expansion (~/), like in your original example, as that's in the scope of the command line shell.

There's also Rel in filepath package, but I don't think it supports ../ in @blancharda case https://cs.opensource.google/go/go/+/go1.18:src/path/filepath/path.go;l=266

Edit: actually filepath.Rel checks for ../ too.

budimanjojo avatar Mar 25 '22 15:03 budimanjojo

We have different keys for each project, but sops was using only the key for the project company because it matched with the prefix path ~/code/company.

Gotcha, appreciate the context. We have a very similar layout and were able to address the issue with a more specific regex (i.e. company/<subFoldersToMatch>/.* or company/<filesToMatch> rather than just company/.*). But I get that it might not be that simple in every case.

blancharda avatar Mar 29 '22 15:03 blancharda

I encountered same problem while running sops -e -i .sops.yaml from root directory. I'm using SOPS v3.7.3 on mac M1 machine.

Solution, as proposed here, is to encrypt it in-place: sops --age=<public-age-key> \ --encrypt --encrypted-regex '^(data|stringData)$' --in-place <path/sub-path/file_to_be_encrypted.yaml>

vsramalwan avatar Apr 21 '23 10:04 vsramalwan