go-witness icon indicating copy to clipboard operation
go-witness copied to clipboard

Product include and exclude glob is not working correctly.

Open matglas opened this issue 1 year ago • 8 comments

After some investigation I found that the Attest part of the Product attestor does not exclude or include items specifically. It is taking into account the items when building the list of subjects that is used for creating a subject collection. But it does not check these products during the creation of the attestation.

I test bash script creating different outputs and using the exclude / include glob arguments. But running the code thru the debugger its not using these values during the creation of the Product attestation.

matglas avatar Nov 13 '23 08:11 matglas

Thank @matglas for identifying and creating this issue. Could you provide some screenshots of output that you are receiving?

tannerjones4075 avatar Dec 04 '23 17:12 tannerjones4075

Sure. I run

witness run --attestor-product-include-glob artifact --step build -o test-att.json -- bash build.sh

File: build.sh

#!/usr/bin/env bash

echo "hello" > artifact

mkdir -p out/new
echo "new" > out/new/.config

Got the following product attestation or better yet, the predicate.

{
  "type": "https://witness.dev/attestations/product/v0.1",
  "attestation": {
    "artifact": {
      "mime_type": "application/octet-stream",
      "digest": {
        "gitoid:sha1": "gitoid:blob:sha1:ce013625030ba8dba906f756967f9e9ca394464a",
        "gitoid:sha256": "gitoid:blob:sha256:473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813",
        "sha256": "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"
      }
    },
    "out/new/.config": {
      "mime_type": "",
      "digest": {
        "gitoid:sha1": "gitoid:blob:sha1:3e757656cf36eca53338e520d134963a44f793f8",
        "gitoid:sha256": "gitoid:blob:sha256:473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813",
        "sha256": "7aa7a5359173d05b63cfd682e3c38487f3cb4f7f1d60659fe59fab1505977d4c"
      }
    }
  },
  "starttime": "2023-12-05T14:09:41.199032+01:00",
  "endtime": "2023-12-05T14:09:41.324371+01:00"
}

I expected

{
  "type": "https://witness.dev/attestations/product/v0.1",
  "attestation": {
    "artifact": {
      "mime_type": "application/octet-stream",
      "digest": {
        "gitoid:sha1": "gitoid:blob:sha1:ce013625030ba8dba906f756967f9e9ca394464a",
        "gitoid:sha256": "gitoid:blob:sha256:473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813",
        "sha256": "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"
      }
    }
  },
  "starttime": "2023-12-05T14:09:41.199032+01:00",
  "endtime": "2023-12-05T14:09:41.324371+01:00"
}

matglas avatar Dec 05 '23 13:12 matglas

Adding some details from a conversation I had with @mikhailswift.

One of the issues with the flagrante currently is that the attestation hides the fact that the flag was used. A policy would not be able to reason about things 'missing' or at least decide what it thinks about it.

If possible could you add your thoughts roo @mikhailswift ?

matglas avatar Mar 08 '24 20:03 matglas

@mikhailswift @jkjell

I want to make a suggestion. Would it be an idea to extend the product attestation in the collection with a configuration part. It could include this type of data about the what is excluded or included.

matglas avatar Apr 08 '24 14:04 matglas

Yes, I think this is a good path forward.

So, we would need to add the includeGlob and exclude glob from the product Attestor struct, to the MarshalJSON and UnmarshalJSON functions. This will allow folks to write policy that allows or disallows certain include or exclude globs. I think this should be a pretty simple addition to the PR you already have.

jkjell avatar Apr 10 '24 18:04 jkjell

Great! I will make that work and do some more testing to verify the logic I implemented works correctly.

matglas avatar Apr 10 '24 18:04 matglas

@jkjell I implemented some changes. I do realize that it might be needed to implement the exclude too for material. Because if I am not going to exclude the same things on the next step of the build it could be a problem too. The issue I am addressing is also regarding size and not including unneeded things. So that should also apply to material.

I'm curious to hear your thoughts. I can implement it there too.

matglas avatar Apr 12 '24 10:04 matglas

☝️ This new PR is an alternative approach for the original problem I had with too big attestations for big amount of files. Files where 2GB. In this implementation I create dirhash for specific directories.

matglas avatar Apr 29 '24 11:04 matglas