non unique dictionary key error

[colek42]

General information:

main
linux
1.19.1

failed to create link metadata: left stripping has resulted in non unique dictionary key: /node_modules/@actions/tool-cache/node_modules/uuid/bin/uuid

When testing the in-toto run action we run into the above error when calculating the materials for the job.

[adityasaky]

Interesting. This error is triggered by a check that kicks in when applying left-strip patterns to see if a left strip results in a collision in artifact paths. Specifically here, it seems to be /node_modules/@actions/tool-cache/node_modules/uuid/bin/uuid. I'm not sure why you're running into something at that path twice given the materials and products you're trying to record though. I wonder if it's the runDir causing problems.

[adityasaky]

Let me experiment with it a bit.

[adityasaky]

Also, you've excluded node_modules but looks like that didn't take.

[adityasaky]

Okay, so I don't yet have an answer for why the exclude didn't work but the issue was https://github.com/testifysec/intoto-run-action/blob/main/node_modules/%40actions/tool-cache/node_modules/.bin/uuid. There was a symlink pointing to an artifact that was already recorded. We should clear up the error message to indicate it can happen even in non left strip situations. cc @shibumi

[adityasaky]

So -e / --exclude works with a trailing slash. The python impl doesn't require that slash, we should ticketize this separately. For now, adding the trailing slash should solve the issue entirely since the symlinks won't be encountered.

TODO:

1. Ticket for required trailing slash in -golang's exclude pattern
2. Symlinks should record the hash of the target but the artifact path should be of the link, not the target

[colek42]

Okay, so I don't yet have an answer for why the exclude didn't work but the issue was https://github.com/testifysec/intoto-run-action/blob/main/node_modules/%40actions/tool-cache/node_modules/.bin/uuid. There was a symlink pointing to an artifact that was already recorded. We should clear up the error message to indicate it can happen even in non left strip situations. cc @shibumi

Should we change this from an error to a warning and continue?

[adityasaky]

I think the problem is that with symlink management, the destination path is used. The file at the destination should be hashed but the path should be the path to the symlink itself.

foo
bar -> foo

When recording both, the paths should be foo and bar with both entries recording the same hash. This is how the python implementation behaves too. This would avoid the left-strip error condition from occurring.

As for warning vs error in the general sense, I think not. With left strips, we can have collisions with entirely different files, where only one will be recorded. The short term resolution for this issue is to exclude node_modules with a trailing slash. Long term, we should consolidate exclude pattern handling and fix the path recorded for symlinks.

[adityasaky]

I believe the exclude patterns issue might belong in @shibumi's go-pathspec.

[adityasaky]

I'm closing this because of the updated symlink handling in #194. @shibumi I'd appreciate your thoughts on the pathspec bits but that can be handled separately!