Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define tool component object to represent tool driver and its extensions/plugins #179

Closed
michaelcfanning opened this issue May 25, 2018 · 12 comments
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. change-draft-available design-approved The TC approved the design and I can write the change draft e-ballot e-ballot-2 enhancement impact-non-breaking-change merged Changes merged into provisional draft. p1 Priority 1 issue to close

Comments

@michaelcfanning
Copy link
Contributor

In order to effectively compare two equivalent tool runs expressed as SARIF, it is useful to verify whether the log files were produced by tool configuration with equivalent functional behaviors.

The tool version provides some help here. The tool invocation data also provides some assistance, in cases where things like the command-line arguments are exactly equivalent.

Tools can be parameterized by things referred to on the command-line (such as scripts that drive analysis, or plug-ins that are delay-loaded). These things can be versioned out-of-sync with the tool.

Additionally, some tools discover plug-ins at runtime, from well-known locations (that aren't explicit on a command-line).

It is difficult or impossible to specify SARIF that can accommodate the range of tool behaviors. But have we done enough on the problem of handling rule scripts/plug-ins versioning that is typical of tools such as Semmle? Fortify also has flexible rules customization/authoring. et al.

@michaelcfanning michaelcfanning added the 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. label May 30, 2018
@ghost
Copy link

ghost commented Aug 24, 2018

@michaelcfanning:

Proposal (copied from #207):
Define a plugIn object with properties fileLocation (of type fileLocation) and version (of type string). Give tool a new property plugIns of type "array of plugIn".

Probably the plugIn object would have the same set of versioning properties that tool does: version, semanticVersion, and fileVersion (but not sarifLoggerVersion).

@michaelcfanning
Copy link
Contributor Author

See #258. As part of completing this analysis, we should be sure to cover the issue of how individual plug-ins both provide rules/resources data and allow for probing for localized information.

@michaelcfanning michaelcfanning added the p1 Priority 1 issue to close label Jan 24, 2019
@michaelcfanning
Copy link
Contributor Author

New proposal: provide a tool property that represents a set of file indices to plug-ins. Add a version property to the file object. Now, a tool can reference all its plug-ins. The file information can contain hash and version details to idenify it. Add an optional property to the rule metadata that refers to its container plug-in (by file index). Consider whether we require a new file 'role' to document this case.

@michaelcfanning
Copy link
Contributor Author

michaelcfanning commented Jan 24, 2019

Remove tool.sarifLoggerVersion. The tool object now contains:

  • language:string
  • driver:toolComponent
  • extensions:toolComponent[]

toolComponent can represent a plugin, but also an auxilliary file such as an app.config.

toolComponent takes the version properties from the current tool object (except for sarifLoggerVersion). Those version properties make sense for plugin DLLs, possibly not for auxilliary files.

@michaelcfanning michaelcfanning added the design-approved The TC approved the design and I can write the change draft label Jan 24, 2019
@michaelcfanning
Copy link
Contributor Author

@lgolding, one thing to keep in mind here is that we've moved rules metadata to the tool object. if this data is now part of tool component, then a result ruleIndex needs to know what component it is associated with. this implies a toolComponentIndex on each result (that can't point away from the driver into some specific plug-in or component). The absence of this property would indicate that the driver component rulesMetadata s/be examined to retrieve rule data.

@ghost
Copy link

ghost commented Feb 11, 2019

@michaelcfanning I agree with your idea for toolComponentIndex.

@michaelcfanning
Copy link
Contributor Author

michaelcfanning commented Feb 11, 2019

Schema changes:

Tool.language retained.
Tool.sarifLoggerVersion deleted
Remainder of tool properties bundled into a toolComponent object.
toolComponent.fileIndex added to that type.
tool.driver is a toolComponent instance and is a required property
tool.extensions is an array of tool components that extend the default tool
result.ruleExtensionIndex can be used to associate a result with an extension
'toolComponent' added as a new file role
conversion.tool is a toolComponent instance (not a tool instance) and is renamed to conversion.driver

@michaelcfanning
Copy link
Contributor Author

Checked into SARIF SDK schema.

ghost pushed a commit that referenced this issue Feb 13, 2019
ghost pushed a commit that referenced this issue Feb 13, 2019
ghost pushed a commit that referenced this issue Feb 14, 2019
ghost pushed a commit that referenced this issue Feb 14, 2019
conversion.tool is the conversion scenario analog of run.tool.
Both the direct production and the conversion scenarios are
accomplished by a tool that consists of a driver, and possibly
a set of auxiliary files including plugins and configuration files.
In the conversion scenario, configuration files are more likely
than plugins, but that’s neither here nor there.
ghost pushed a commit that referenced this issue Feb 18, 2019
@michaelcfanning
Copy link
Contributor Author

Please note update to this issue, result.extensionIndex should be named result.ruleExtensionIndex (to provide parallelization with result.ruleIndex...)

@michaelcfanning
Copy link
Contributor Author

michaelcfanning commented Feb 27, 2019

EBALLOT PROPOSAL: define a mechanism to allow a tool to explicitly detail any plug-ins or extensions that were in play during the analysis run (and which potentially contribute to changes in analysis behavior).

API IMPACT

  • tool.sarifLoggerVersion property deleted (as it hasn't been helpful to date)
  • Define a toolComponent object to represent a tool's driver and its extensions.
    • Transfer all existing tool properties except language to toolComponent, with the following changes:.
      • Change the type of the shortDescription property from message object to multiformatMessageString object.
      • Change the type of the fullDescription property from message object to multiformatMessageString object.
    • Add a property artifactIndex to locate the component in the run.artifacts array.
  • In the tool object:
    • Add a property driver of type toolComponent to represent the driver: required
    • Add a property extensions of type array of toolComponent to represent all extensions.
  • Add toolComponent as a new file role.

NOTES

  1. Since conversion.tool was and remains a tool instance, a converter has a driver and can have extensions.
  2. result.rulePointer can be used to associate a result with an extension (see Define a reportingDescriptorReference object #324 for more)

POST-BALLOT NOTE

@michaelcfanning michaelcfanning changed the title Consider whether SARIF covers plug-ins/rules versioning sufficiently Define tool component object to represent tool driver and its extensions/plugins Feb 27, 2019
ghost pushed a commit that referenced this issue Mar 5, 2019
@ghost
Copy link

ghost commented Mar 17, 2019

E-BALLOT #2 PROPOSAL

We decompose a tool object into a number of toolComponents: a "driver" (the component that contains the tool's primary executable) and zero or more "extensions" (components that affect the tool's behavior, such as plugins and configuration files).

This ballot proposal incorporates the later changes introduced by #336 ("Changes to toolComponent properties").

SCHEMA CHANGES

  • In the tool object:

    • Remove the never-used sarifLoggerVersion property.
    • Add a property driver of type toolComponent, required.
    • Add a property extensions of type toolComponent[], optional, minItems: 0, default: empty array
    • Move all other properties to the toolComponent object (name, fullName, version, semanticVersion, dottedQuadFileVersion, and downloadUri) to the toolComponent object (defined next).
  • Define a toolComponent object with the following properties:

    • All the properties moved from the tool object (name, fullName, version, semanticVersion, dottedQuadFileVersion, and downloadUri).
    • releaseDateUtc of type string in date-time format -- the component's release date.
    • informationUri of type string in uri format, optional.
    • organization of type string, optional -- the company or organization that produced the component, e.g., "Example Corporation".
    • product of type string, optional -- the name of the product containing the component, e.g., "Example Corp SecurityScanner".
    • productSuite of type string, optional -- the name of the suite of products containing the component, e.g., "Example Corp Code Quality Tools".
    • shortDescription of type multiformatMessageString, optional -- a brief description of the component.
    • fullDescription of type multiformatMessageString, optional -- a comprehensive description of the component.
    • artifactIndices of type integer[], optional, default: empty array -- references to the files which comprise the component.

NOTES

  1. Contrary to earlier comments in this issue, we will not yet move rules from resources to toolComponent, and so we will not yet introduce toolComponentIndex. Those concepts were originally introduced in Define a reportingDescriptorReference object #324. They will appear in an upcoming change draft that includes Provide full metadata objects for notifications #311 (notification metadata), Define a reportingDescriptorReference object #324 (reportingDescriptorReference), and Remove current localization mechanism #325 (Remove old localization design).

ghost pushed a commit that referenced this issue Mar 17, 2019
@ghost ghost self-assigned this Mar 17, 2019
@ghost ghost added change-draft-available merged Changes merged into provisional draft. labels Mar 17, 2019
@ghost ghost added the e-ballot-2 label Apr 3, 2019
@ghost
Copy link

ghost commented Apr 6, 2019

Neglected to close this after e-ballot-2.

@ghost ghost closed this as completed Apr 6, 2019
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Aug 27, 2019
This updates the SARIF exporter to produce SARIF 2.1 output. The bulk of the diffs come from two changes to SARIF:
* oasis-tcs/sarif-spec#309
* oasis-tcs/sarif-spec#179

Differential Revision: https://reviews.llvm.org/D65211

llvm-svn: 370068
spurious pushed a commit to spurious/clang-mirror that referenced this issue Aug 27, 2019
This updates the SARIF exporter to produce SARIF 2.1 output. The bulk of the diffs come from two changes to SARIF:
* oasis-tcs/sarif-spec#309
* oasis-tcs/sarif-spec#179

Differential Revision: https://reviews.llvm.org/D65211


git-svn-id: http://llvm.org/svn/llvm-project/cfe/trunk@370068 91177308-0d34-0410-b5e6-96231b3b80d8
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. change-draft-available design-approved The TC approved the design and I can write the change draft e-ballot e-ballot-2 enhancement impact-non-breaking-change merged Changes merged into provisional draft. p1 Priority 1 issue to close
Projects
None yet
Development

No branches or pull requests

1 participant