Make certain invocation and versionControlDetails properties redactable

[ghost]

NOTE: This description comment is OBSOLETE. See the REVISED PROPOSAL below.

Per feedback from Ykaterina and validated by discussion with @michaelcfanning:

Make the following properties of the invocation object redactable:

• machine (replace the entire string with a redaction token)
• account (replace the entire string with a redaction token)
• processId (replace the entire string with a redaction token)
• executableLocation (uri property is redactable; that is, replace selected segments of the uri with a redaction token)
• workingDirectory (uri property is redactable; that is, replace selected segments of the uri with a redaction token)
• environmentVariables (replace selected property names with distinct redaction tokens; replace selected portions of property values with redaction tokens)

NOTE: To cover executableLocation and workingDirectory, we should state in the section about artifactLocation.uri that any uri is redactable.

Consider making the following properties of the versionControlDetails object redactable:

• repositoryUri
• revisionId
• branch
• revisionTag

UPDATES

• inovcation.processId is an integer, so not redactable.
• We now allow artifactLocation.uri to be redactable, which covers invocation.executableLocation and invocation.workingDirectory.
• In fact the spec now says that every URI-valued string (even if it doesn't occur as a value of artifactLocation.uri) is redactable.

[kupsch]

The uri value for a nested file should not be required to start with a an absolute path as was added to this draft (I had commented on this earlier). Both tar and zip support relative paths and absolute paths in the same archive. This means that an archive can contain both "/foo.txt" and "foo.txt" and that these are distinct archive member with distinct values. If you require absolute paths how are the two distinguished. The guidance should be to have the uri (not a uri, but the member name) use the member name or path as is from the archive. Generally this will be a relative path as this is the most commonly used type.

[michaelcfanning]

This makes sense. It’s certainly true an archive can contain both these references. Larry, can you see how disruptive this is to the spec? Basically, any producer in this scenario needs to consider the presence or non-presence of a leading slash carefully. A note re: the archive scenario (which is the prominent driver for the feature) will help. This is a good catch, thanks, Jim. Michael From: kupsch <notifications@github.com> Sent: Friday, April 26, 2019 11:40 AM To: oasis-tcs/sarif-spec <sarif-spec@noreply.github.com> Cc: Michael Fanning <Michael.Fanning@microsoft.com>; Mention <mention@noreply.github.com> Subject: Re: [oasis-tcs/sarif-spec] Make certain invocation and versionControlDetails properties redactable (#390) The uri value for a nested file should not be required to start with a an absolute path as was added to this draft (I had commented on this earlier). Both tar and zip support relative paths and absolute paths in the same archive. This means that an archive can contain both "/foo.txt" and "foo.txt" and that these are distinct archive member with distinct values. If you require absolute paths how are the two distinguished. The guidance should be to have the uri (not a uri, but the member name) use the member name or path as is from the archive. Generally this will be a relative path as this is the most commonly used type. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fissues%2F390%23issuecomment-487159876&data=01%7C01%7CMichael.Fanning%40microsoft.com%7C9cdc58bbfa1b4a7a6d4308d6ca768686%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=12iac3Jy284tVkwk5bcUczN74CU%2Fk5GgeBzsqX7BocI%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADHGLN5G6GIKXQ6VWMNEXYLPSND6HANCNFSM4HHALJEQ&data=01%7C01%7CMichael.Fanning%40microsoft.com%7C9cdc58bbfa1b4a7a6d4308d6ca768686%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=nLoxUQRw7eqskXCkiw1TdoMnPL1LDVYDx%2FaEFsRBfvY%3D&reserved=0>.

[michaelcfanning]

It is absolutely not disruptive. I had removed that sentence months ago - I thought accidentally, but now that Jim mentions, it might have been deliberate based on his feedback - and it costs me nothing to remove it again. Thanks for catching that, Jim! From: Michael Fanning <Michael.Fanning@microsoft.com> Sent: Friday, April 26, 2019 11:49 AM To: oasis-tcs/sarif-spec <reply@reply.github.com>; oasis-tcs/sarif-spec <sarif-spec@noreply.github.com> Cc: Mention <mention@noreply.github.com>; Larry Golding (Myriad Consulting Inc) <v-lgold@microsoft.com> Subject: RE: [oasis-tcs/sarif-spec] Make certain invocation and versionControlDetails properties redactable (#390) This makes sense. It's certainly true an archive can contain both these references. Larry, can you see how disruptive this is to the spec? Basically, any producer in this scenario needs to consider the presence or non-presence of a leading slash carefully. A note re: the archive scenario (which is the prominent driver for the feature) will help. This is a good catch, thanks, Jim. Michael From: kupsch <notifications@github.com<mailto:notifications@github.com>> Sent: Friday, April 26, 2019 11:40 AM To: oasis-tcs/sarif-spec <sarif-spec@noreply.github.com<mailto:sarif-spec@noreply.github.com>> Cc: Michael Fanning <Michael.Fanning@microsoft.com<mailto:Michael.Fanning@microsoft.com>>; Mention <mention@noreply.github.com<mailto:mention@noreply.github.com>> Subject: Re: [oasis-tcs/sarif-spec] Make certain invocation and versionControlDetails properties redactable (#390) The uri value for a nested file should not be required to start with a an absolute path as was added to this draft (I had commented on this earlier). Both tar and zip support relative paths and absolute paths in the same archive. This means that an archive can contain both "/foo.txt" and "foo.txt" and that these are distinct archive member with distinct values. If you require absolute paths how are the two distinguished. The guidance should be to have the uri (not a uri, but the member name) use the member name or path as is from the archive. Generally this will be a relative path as this is the most commonly used type. - You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fissues%2F390%23issuecomment-487159876&data=01%7C01%7Cv-lgold%40microsoft.com%7C4c3b831158c546076b6708d6ca77d314%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=TZ7vjgTg6lwsW0ixDbCkmC5CkZFpEDbBhzpGpP845S0%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADHGLN5G6GIKXQ6VWMNEXYLPSND6HANCNFSM4HHALJEQ&data=01%7C01%7Cv-lgold%40microsoft.com%7C4c3b831158c546076b6708d6ca77d314%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=oetlW35LwLEsc7bU5AHwLn78uPsbKAOuZr2xlBFm%2Blg%3D&reserved=0>.

[ghost]

REVISED PROPOSAL

Make the following properties of the invocation object redactable:

• machine
• account
• environmentVariables (replace selected property names with distinct redaction tokens; replace selected portions of property values with redaction tokens)

Make the following properties of the versionControlDetails object redactable:

• revisionId
• branch
• revisionTag

Make artifactLocation.uri optional (with no default value) in the specific case where the artifactLocation is a top-level value in originalUriBaseIds. This improves the usefulness of originalUriBaseIds in representing sanitized URIs. That is, given this:

"originalUriBaseIds": {
 "PROJECTROOT": {
    "uri": "file:///C:/Users/Larry/code/OurProject",
    "description": "The root directory for all project files."
 },
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

... we will now be able to write this:

"originalUriBaseIds": {
 "PROJECTROOT": {
    "description": "The root directory for all project files. (Look, no uri!)"
 },
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

... rather than having to remove the PROJECTROOT property entirely, like this:

"originalUriBaseIds": {
  "SRCROOT": {
    "uri": "src",
    "uriBaseId": "PROJECTROOT",
    "description": "The root of the source tree."
  }
}

We get the security benefit without losing the documentation for the uriBaseId value.

[ghost]

This became a schema change when we allowed a top-level artifactLocation object in originalUriBaseIds to have neither a uri nor an index. So I removed the anyOf that required one of those two properties.