`reportingDescriptor` `name` should mention pascal style constraint

[jsoref]

trafficstars

Code of Conduct

• [X] I have read and agree to the GitHub Docs project's Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#reportingdescriptor-object

What part(s) of the article would you like to see updated?

Name	Description
name	Optional. The name of the rule. Code scanning displays the name to allow results to be filtered by rule on GitHub.

Should mention:

SARIF2012: Rule metadata should provide information that makes it easy to understand and fix the problem. Provide the 'name' property, which contains a "friendly name" that helps users see at a glance the purpose of the rule. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal-case identifier, for example, 'ProvideRuleFriendlyName'. Provide the 'helpUri' property, which contains a URI where users can find detailed information about the rule. This information should include a detailed description of the invalid pattern, an explanation of why the pattern is poor practice (particularly in contexts such as security or accessibility where driving considerations might not be readily apparent), guidance for resolving the problem (including describing circumstances in which ignoring the problem altogether might be appropriate), examples of invalid and valid patterns, and special considerations (such as noting when a violation should never be ignored or suppressed, noting when a violation could cause downstream tool noise, and noting when a rule can be configured in some way to refine or alter the analysis).

Additional information

Through a couple of levels of indirection, this page links to this page: https://sarifweb.azurewebsites.net/Validation

And when I tried to validate my sarif file (checking the GitHub ingestion rules option), the validator screamed at me.

See steps to reproduce in: https://github.com/github/docs/issues/20872#issuecomment-1534850272

---

Details of a separate problem...

Each item in this page should mention length and other constraints as well...

Name	Description
name	Optional. The name of the rule. Code scanning displays the name to allow results to be filtered by rule on GitHub.
shortDescription.text	Required. A concise description of the rule. Code scanning displays the short description on GitHub next to the associated results.
fullDescription.text	Required. A description of the rule. Code scanning displays the full description on GitHub next to the associated results. The max number of characters is limited to 1000.

fullDestription.text is limited to 1000 chars, but name and shortDescription.text aren't limited?

---

Content changes required to close this issue

1. Update the description for the name field for the reportingDescriptor object to state that there are no restrictions on the format of the string.
2. Add a note to the section on Validating your SARIF file:
   • Tell people that if there is a discrepancy between the SARIF validator results and the specification, the specification is correct.
   • Try uploading their SARIF file using the REST API with the validation flag.
   • When the validation flag is enabled, any validation errors are returned as the response body, with an error code immediately. This is a good method for debugging any errors that aren't detected by the SARIF validator, or cases where the validator and schema contradict each other.

[cmwilson21]

👋 @jsoref - Thanks so much for opening an issue! I'll triage this for the team to take a look :eyes:

[felicitymay]

Thanks for flagging this @jsoref - much appreciated.

Each item in this page should mention length and other constraints as well...

I'm going to tag this for an SME to take a look and advise on this. Then we'll add update the issue summary with details of the content changes needed.

[github-actions[bot]]

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert :eyes:

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[simon-engledew]

Each item in this page should mention length and other constraints as well...

The limits on the three fields in that table are:

• Name - 255 characters
• ShortDescription.Text - 1024 characters
• FullDescription.Text - 1024 characters

[jsoref]

@felicitymay: this covers part two of the problem (character limits for 3 fields) but not part one (Pascal Style):

For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal-case identifier, for example, 'ProvideRuleFriendlyName'.

[felicitymay]

Thanks for the reminder @jsoref

When I searched for the text you quoted in the issue summary in http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html, I failed to find it. Could you check that it's still there?

Looking at "3.49.7 name property", I can see that they use a Pascal example name but that they don't specify that this is a requirement.

The longer example SARIF file in the article seems to include name that is not Pascal-case, so I'm wondering if this might have been an error in the specification that's since been corrected 🤔

[jsoref]

Steps:

1. Download https://raw.githubusercontent.com/jsoref/sarif-file-conventions/main/sample2.sarif.json
2. Visit: https://sarifweb.azurewebsites.net/Validation
   • [x] GitHub ingestion rules
3. Upload sample2.sarif.json

It highlights:

          "name": "Unrecognized-Spelling2",

And says:

SARIF2012: runs[0].tool.driver.rules[0].name: 'Unrecognized-Spelling2' is not a Pascal-case identifier. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal-case identifier, for example, 'ProvideRuleFriendlyName'.

It's actually complaining 5 times, you can use the down arrow to discover a similar complaint for each of the instances, although frustratingly it doesn't like EOF, I didn't realize it wouldn't like that...

Note that: https://sarifweb.azurewebsites.net/

Collaborators

The SARIF project is supported by a group of industry contributors.

• Contrast
• Sonar
• ForAllSecure
• Mend
• GrammaTech
• Micro Focus
• Microsoft
• GitHub, Inc.

...

Copyright © Microsoft Corporation. All rights reserved.

If this isn't right, then someone from GitHub (a sponsor of https://sarifweb.azurewebsites.net/) should tell someone from Microsoft (the host and sponsor and owner of GitHub) to change their content.

If it's right, then, ideally the documentation would be fixed to cover this stuff so that I wouldn't have to get it from this validator.

[felicitymay]

@jsoref - many thanks for the clear reproduction steps. I'll tag this for an SME review.

[github-actions[bot]]

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert :eyes:

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[felicitymay]

Hi @jsoref 👋🏻

Apologies for the delay in getting back to you.

I can confirm that there is no need to use Pascal case for the reportingDescriptor name property. The SARIF specification sets no limits on the text for this field, see 3.49.7 name property. I agree with you that the example is misleading.

Proposed actions:

• Let Microsoft know that they need to update the validation for GitHub ingestion rules.
• Update the description for this field to state that there are no restrictions on the format of the string.
• Add a note to the section on validating your SARIF file to let people know that if there is a discrepancy between the validator results and the specification, the specification is correct. They should try uploading their SARIF file using the REST API with the validation flag. When the validation flag is enabled, any validation errors are returned as the response body, with an error code immediately. This allows for faster debugging.

I think this will make this clearer in the docs and hopefully the validator will be updated soon.

[jsoref]

I don't see the validation flag in the REST documentation?

[felicitymay]

I didn't find it easily either. It's the last body parameter:

[jsoref]

Oh, I think either your anchor was for an earlier object or my GitHub Mobile for Android is not doing me any favors. In theory the link should be to:

https://docs.github.com/en/rest/code-scanning/code-scanning?apiVersion=2022-11-28#upload-an-analysis-as-sarif-data

Disclaimer: I'm using GitHub Mobile for Android beta, and it's possible that it's doing a bad job of jumping to the anchor.

Specifically, this is what I see when I click the link:

Reported as https://github.com/orgs/community/discussions/67482

Do note that it would be really helpful to be able to link to parameters either as a section or as individual items, and neither are easy to do today.

[felicitymay]

Interesting. That is the same link that I gave.

I can certainly raise a request to look into the possibility of making more anchors available on the page. I suspect that there may be concerns with adding more visual distraction to the page, but the only way to find out is to ask.

[jsoref]

I can understand the concern about visual clutter. If the additional anchors didn't have the magic visual distinctions and were only discoverable via DOM Inspection, I'd still take the half-improvement as people like you and me could produce them (using a typical desktop web browser) and everyone could benefit from consuming them (as long as our HTML user agent isn't broken – see discussion linked from edited comment above).

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[jsoref]

@cmwilson21

[nguyenalex836]

Reopened