Add OriginalUriBaseIds sample. #14

ghost · 2020-04-24T00:48:03Z

No description provided.

michaelcfanning · 2020-04-24T18:44:54Z

We've received a new request that I think makes a lot of sense. We should be producing documentation that describes these test assets and how they should be used by consumers. The suppressions sample, for example, should explicitly call out that this data is designed to ensure that viewers correctly exclude or include suppressed results (and don't surprise users by showing them in a way that suggests they are active and should be addressed.

This will allow viewer developers to try to exercise the test asset in a productive way without understanding all the many nuances expressed in the SARIF itself (which is only obvious to people like us who have a deep understanding of the spec). To bring this to the current example, the original uri base ids should be helpful for viewers that are attempting to remap relative links in a SARIF log file. If a log is viewed in an environment that maps directly to the paths expressed in an original uri base id (i.e., the local enlistment matches the analysis or the local machine was itself used for the scan), consumers should be able to successfully stitch together working links, using other data expressed in the physical location data. Etc. #Pending

michaelcfanning · 2020-04-24T18:48:08Z

samples/OriginalUriBaseIds.sarif

+      "originalUriBaseIds": {
+        "PROJECTROOT": {
+          "description": {
+            "text": "The root directory for all project files."


for all project files [](start = 40, length = 21)

Confusing example. most windows project files have their own directory, and therefore each project file might have its own root (it does in visual studio). You might change this to SOLUTIONROOT, of which we can more reasonably expect there to be a single definition for a scan run). #Resolved

Renamed to REPOROOT, and re-described, per your comment below.

In reply to: 414790002 [](ancestors = 414790002)

michaelcfanning · 2020-04-24T18:50:29Z

samples/OriginalUriBaseIds.sarif

+        },
+        "SRCROOT": {
+          "uri": "src",
+          "uriBaseId": "PROJECTROOT",


PROJECTROOT [](start = 24, length = 11)

Ah, now I understand, by project root you mean, ENLISTMENTROOT. Suggest making that change. Or perhaps use REPOROOT . Project is just too overloaded a term in MS dev tooling to provide a clear example here. To an old MS workhorse like me, that is. :P #Resolved

Renamed to REPOROOT.

In reply to: 414791357 [](ancestors = 414791357)

michaelcfanning · 2020-04-24T18:52:43Z

samples/OriginalUriBaseIds.sarif

+      },
+      "results": [
+        {
+          "ruleId": "SMP0001",


SMP0001 [](start = 21, length = 7)

How is SMP pronounced? 'simp'? if so, unfortunate. Also 'too many zeros'. :)

How about a convention of SAMPXXX for the work we do here? I know this is a bit of a departure from conventions used by tools that reasonable allocation a range of 9999 potential issues. I think the adjustment helps with readability here. i.e., I like 'SAMP001' better than 'SMP0001'. Thoughts? This is nearly entirely in the realm of opinion... #Pending

I'm SMPathetic to your concern about SMP, but I don't want to model, in our entire corpus of test assets, a pattern that we don't encourage tool authors to follow. How about TST0001, which I think you'll agree is obviously pronounced "test"?

Whatever we decide to do, I'll do in a separate PR that fixes all the files at once.

In reply to: 414792603 [](ancestors = 414792603)

There's no rigid standard here. Honestly, your string of zeros compromises readability and if you are adhering to some standard tooling convention, we don't typically start indexing from 1, these lower-level ids are almost always reserved for internal error codes. CS1002 is more typical.

How about TUT1001, TUT1002, etc. or TST1001, TST1002, etc.

In reply to: 414849147 [](ancestors = 414849147,414792603)

Sold! I filed #15, "Change all ruleIds to TUT1nnn, e.g., TUT1001".

In reply to: 414852413 [](ancestors = 414852413,414849147,414792603)

michaelcfanning · 2020-04-24T18:55:40Z

samples/OriginalUriBaseIds.sarif

+        {
+          "ruleId": "SMP0002",
+          "message": {
+            "text": "A result in a source file."


source file [](start = 35, length = 11)

hm, shall we power up this sample and add a reference to a directory in addition to distinct files? is this a concern that belongs in a different sample? #Resolved

Good idea, done.

In reply to: 414794276 [](ancestors = 414794276)

michaelcfanning · 2020-04-24T19:32:09Z

samples/OriginalUriBaseIds.sarif

+          }
+        },
+        "SRCROOT": {
+          "uri": "src",


src [](start = 18, length = 3)

Is this legal? shouldn't it end in a forward slash? do we have a hole in our validator (or maybe you haven't authored unit tests for these assets that validate?) btw - if this is a bug it's in our spec as well, which i'm looking at #Resolved

You're right. I've been running sarif validate on the sample files, but my installed copy doesn't yet have the "trailing slash" fix. I'll switch to using my private bits.

In reply to: 414813884 [](ancestors = 414813884)

... aaaaand I just found a bug in the trailing slash analysis. I'll file an SDK bug and push a PR.

In reply to: 414824105 [](ancestors = 414824105,414813884)

Filed and fixed the SDK bug, raised a PR on the SDK, added the slash here.

In reply to: 414829741 [](ancestors = 414829741,414824105,414813884)

michaelcfanning · 2020-04-24T19:33:19Z

samples/OriginalUriBaseIds.sarif

+          "name": "SarifSamples"
+        }
+      },
+      "originalUriBaseIds": {


originalUriBaseIds [](start = 7, length = 18)

you don't have an example in here that resolves to an absolute URL #Resolved

I added an example (LOGSROOT resolving directly to [file:///C:/logs/](file:///C:/logs/`)).

In reply to: 414814508 [](ancestors = 414814508)

michaelcfanning · 2020-04-24T19:34:03Z

samples/OriginalUriBaseIds.sarif

+{
+  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
+  "version": "2.1.0",
+  "properties": {


properties [](start = 3, length = 10)

btw - original url base ids shouldn't create 'loops'. since we have a dedicated 'don't screw up original uri base ids' rule, we could add this verification to that check. #Pending

We're about to have one of our partitioning discussions. Rule SARIF1018 is about the validity of the URIs in originalUriBaseIds: They have to end with a slash, and if there's no uriBaseId, they have to be absolute. The (good) rule you're proposing is about the structure of the uriBaseIds. I'll make it part of 1018 if you want. But do consider how you would (1) friendly-name, and (2) describe, the combined rule.

In reply to: 414814948 [](ancestors = 414814948)

I filed microsoft/sarif-sdk#1864, "Analysis rule: uriBaseIds should not form loops". In the description I explicitly say that we haven't decided whether this is a separate rule or an enhancement to an existing one.

In reply to: 414865952 [](ancestors = 414865952,414814948)

michaelcfanning · 2020-04-24T19:35:05Z

approved with several suggestions. ✌️

ghost · 2020-04-24T21:35:05Z

I filed #16, "Document usage of samples".

In reply to: 619182186 [](ancestors = 619182186)

Add OriginalUriBaseIds sample.

5734ead

ghost requested a review from michaelcfanning April 24, 2020 00:48

michaelcfanning reviewed Apr 24, 2020

View reviewed changes

ghost merged commit 4c0e46b into master Apr 24, 2020

ghost deleted the users/lgolding/original-uribaseids-sample branch April 24, 2020 21:45

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OriginalUriBaseIds sample. #14

Add OriginalUriBaseIds sample. #14

ghost commented Apr 24, 2020

michaelcfanning commented Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

michaelcfanning Apr 24, 2020

ghost Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

ghost Apr 24, 2020

ghost Apr 24, 2020

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020

ghost Apr 24, 2020

michaelcfanning commented Apr 24, 2020

ghost commented Apr 24, 2020

Add OriginalUriBaseIds sample. #14

Add OriginalUriBaseIds sample. #14

Conversation

ghost commented Apr 24, 2020

michaelcfanning commented Apr 24, 2020 • edited by ghost Loading

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning Apr 24, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning commented Apr 24, 2020

ghost commented Apr 24, 2020

michaelcfanning commented Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

ghost Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading

michaelcfanning Apr 24, 2020 •

edited by ghost

Loading