-
Notifications
You must be signed in to change notification settings - Fork 587
feature: add JSON report to verify command #4607
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
feature: add JSON report to verify command #4607
Conversation
|
Thanks for the PR! This is an important use case - to be able to provide structured verification output for processing by other tools. The way Velociraptor is designed, most of the functionality is exported via VQL and the commands are usually just thin wrappers around that functionality (or at least we aim to make all functionality available via VQL artifacts - it is a work in progress for some things). This approach has the advantage that people can customize the output as they please by just modifying the artifacts if needed. For example in previous versions the VQL makes the artifact verifier available via the verify() function which can be used to build the basic functionality. The output you presented can be produced by the following artifact. name: Server.Utils.ArtifactVerifier
type: SERVER
parameters:
- name: SearchGlob
default: /tmp/*.yaml
description: A glob to capture all artifacts to verify
- name: ErrorIsFatal
type: bool
default: N
description: If set, an error is produced if any artifact is failed.
sources:
- query: |
-- Convert the array to a string
LET _Stringify(X) = SELECT str(str=_value) AS value
FROM foreach(row=X)
LET Stringify(X) = _Stringify(X=X).value
LET MaybeLogError(Verify, Path) = if(condition=ErrorIsFatal,
then=Verify.Errors AND log(level="ERROR", dedup= -1, message="%v failed!", args=Path),
else=NOT Verify.Errors)
-- Extract the name of the artifact from the raw data - needed if
-- the yaml can not be parsed at all then we need to fallback to a
-- regex.
LET GetName(Artifact) = parse_string_with_regex(
regex='''^name:\s*(.+)''', string=Artifact).g1
LET Files = SELECT OSPath,
read_file(filename=OSPath, length=10000) AS Data
FROM glob(globs=SearchGlob)
LET Results <= SELECT name,
path,
MaybeLogError(Verify=Verify, Path=path) AS passed,
Stringify(X=Verify.Errors) AS errors,
Stringify(X=Verify.Warnings) AS warnings
FROM foreach(row=Files,
query={
SELECT OSPath AS path,
GetName(Artifact=Data) AS name,
verify(artifact=Data) AS Verify
FROM scope()
})
-- Add some metadata to the output and present in the same row.
SELECT timestamp(epoch=now()) AS timestamp,
config.version as metadata,
dict(
total=len(list=Results),
passed=len(list=filter(list=Results, condition="x=>x.passed")),
failed=len(list=filter(list=Results, condition="x=>NOT x.passed")),
warnings=len(list=filter(list=Results, condition="x=>x.warnings"))
) AS summary,
{ SELECT name FROM Results } as artifacts,
Results as results
FROM scope()I am not sure if this is the best output or some tweaking is required. I think it is important to have that artifact built in which is why I added it in #4608 but that artifact can be used on older versions using the --definitions flag and the new CLI parser: The idea is to allow artifacts to be used seamlessly as CLI commands so you can extend this artifact by renaming it or overriding it to suite your own requirement. PS Note that with bash, you need to protect the * using single quotes to ensure it makes it to the program and bash does not expand it instead. |
|
Thanks @scudette! I hadn't thought of turning it into an artifact, that totally makes sense with Velociraptor's design and also makes it backwards compatible which is great. Thanks for taking the time to put together this artifact, I look forward to utilizing it and hope that others will find it useful as well! Did you want me to help refactor the Lines 116 to 124 in 04400b5
Thanks again! 😃 |
|
maybe the artifact should log the "Verified ... OK" logs and emit the full row so we can run it with the --output flag to redirect output to the CI artifacts and then the logs will look exactly the same as the current output. |
|
So noticed something kind of funky, I was running the
{
"name": "Server.PostProcess.FluentBit",
"path": "/home/user/github/velociraptor/output/artifacts/exchange/FluentBit.yaml",
"passed": false,
"errors": [
"Invalid artifact Server.PostProcess.FluentBit: All Queries in a source must be LET queries, except for the final one."
],
"warnings": []
}
[
{
"Result": {
"Artifact": "...",
"Permissions": [
"EXECVE",
"FILESYSTEM_WRITE",
"MACHINE_STATE",
"READ_RESULTS"
],
"Errors": null,
"Warnings": null,
"Definitions": "..."
}
}
]I'll have a look into it, see if I can figure it out, just flagging it here though (might also be user error..) Edit: Figured it out, the artifact gets truncated by the velociraptor/artifacts/definitions/Server/Utils/ArtifactVerifier.yaml Lines 42 to 44 in 04400b5
|
|
yes that's right - we probably need a much larger buffer. |
Refactors the `artifacts verify` command to use the `verify` VQL function which better fits Velociraptor's VQL-first design. This is only an initial implementation, it currently does not provide feature parity due to missing control over overriding built-in artifacts in the `verify` function.
Adds a `disable_override` argument to the `verify` function which adds the ability control the override of built-in artifacts. This has been added to maintain backwards compability as previously built-in artifacts were overridden by default.
Removes creation of the local artifact repository during verification to ensure that we can check for override of built-in artifacts.
Re-implements the `--builtin` flag functionality for the `verify` command for feature parity and backwards compatibility.
|
How's this looking, @scudette? I've refactored the This should now hopefully have feature parity and backwards combability to ensure that the behavior stays the same for existing consumers but allows us to continue to modify the functionality. Do we still want to add similar JSON output functionality to the I'll probably also modify the Is it worth creating a new pull request for these features as we've deviated from the original pull request scope? |
|
removing the local repository is actually a problem and it exposes a potential bug. Verifying an artifact we dont necessarily want to import it into the global repository - we just want to verify it. This is why we have a local repository so we can load the artifacts into it without really importing them into the main repository. The issue is that we actually need to try and import all the artifacts in before we verify them because we dont know if any custom artifacts refer to other custom artifacts. Say you have an artifact So the old code (in the artifact verify command) was doing it in two steps:
This actually is harder to do in VQL with the verify() function - if we glob all the files, we need a way to load them into a custom repository first, then verify against that one. So I think we need:
Then the VQL would do it in two passes, first import into the local repository and then verify against it. The function can get the local repository out of the scope using CacheGet() for example: velociraptor/vql/protocols/lambda.go Line 63 in 04400b5
|
|
Ah okay! That's a misunderstanding on my behalf of the original logic; I hadn't considered that case. Let me have another crack at it, I'll see if I can come up with a solution that fixes the dependency issue and also respects the prevention of built-in artifact override. |
|
I've made some initial implementations, although I suspect there are a lot of edge cases I haven't considered yet. Additionally, I still need to implement the built-in artifact functionality as it's not checked at the moment in the new changes. |
|
Just looking at solving this built-in configuration, should we add all of the existing artifact definitions from the global repository to the new local repository so that we can check if the already exist. Or, simply query the global repository before adding to the local repository to see if it's actually a built-in? Edit: I've added this to the if arg.DisableOverride {
tmp_repository := manager.NewRepository()
tmp_repository.SetParent(repository, config_obj)
_, err = tmp_repository.LoadYaml(arg.Artifact,
services.ArtifactOptions{
ValidateArtifact: true,
ArtifactIsBuiltIn: false,
})
if err != nil {
state.SetError(err)
return state
}
}Edit 2: The above doesn't quite work, I think because there's a bit of complexity in that the artifact can be the name of an artifact or it can be YAML/JSON, still trying to work out how to resolve this. |
1. Order of artifact evaluation was non deterministic due to use of map 2. If an artifact was not able to load it would not produce any error. 3. The Server.Utils.ArtifactVerifier did not account for dependencies of other custom artifacts.
|
This looks reasonable 👍🏽 . I played with this PR a bit and found a couple of bugs so I pushes a fix in the latest commit Looking great! |
Thanks for fixing up those issues! I think I've figured out how to re-implement the configurable built-in override functionality by adding the check into the initial call to velociraptor/vql/server/repository.go Lines 113 to 126 in 17aa307
I'm sure there are some weird edge-cases here that I've missed, but I think everything appears to be working now? |
Removed `length` argument from call to `read_file` as it's implicitly set to 4 MiB. This should be enough to cover most artifacts.
Adds a `issues_only` flag to the `verify` command that suppresses output of successful artifact verifications. This can be used to remove noise in the command output.
|
Thanks for working through this PR - it is really good! |


Overview
This pull request adds support for generating a JSON report from the
artifacts verifycommand, along with several quality-of-life improvements to make artifact verification easier to integrate into CI/CD pipelines.The following flags have been added to the
artifacts verifycommand:soft_fail- returns a zero exit code even when verification failures occur, allowing consumers to determine the outcome programmaticallyformat- specifies the output format (currently only json is supported)output- specifies the file path to which the output should be writtenPreviously, determining the result of
artifacts verifyrequired parsing stdout/stderr and extracting the verification outcome from log output. This approach provides a more structured and reliable way to consume results and also makes it easier to pass them to downstream tasks for further processing or conversion (for example, to JUnit or SARIF).The implementation is intentionally designed to be extensible. Longer term, the goal is to support native SARIF output for artifact verification so results can be consumed directly by pipelines that support static analysis reporting. At present, differentiating error types would require parsing error messages and mapping them to rule identifiers, which is possible but fairly involved. This change provides a practical intermediate step while laying the groundwork for that future enhancement.
Please let me know if you have any recommendations or changes to help this feature fit into Velociraptor's design.
Example
The following code snippet is an example output, showcasing successful, warning and failed artifact validations.
{ "timestamp": "2026-01-04T08:44:42Z", "metadata": { "version": "0.75.6", "commit": "c068283e7", "build_time": "2026-01-04T19:44:26+11:00", "command": "./velociraptor artifacts verify -v --soft_fail --format json --output report.json ./artifacts/Artifact1.yaml ./artifacts/Artifact2.yaml ./artifacts/Artifact3.yaml ./artifacts/Artifact4.yaml" }, "exit_code": 0, "summary": { "total": 4, "passed": 1, "warning": 1, "failed": 2 }, "artifacts": [ "Generic.Test.Artifact1", "Generic.Test.Artifact2", "Generic.Test.Artifact3", "Unknown" ], "results": [ { "name": "Generic.Test.Artifact1", "path": "./artifacts/Artifact1.yaml", "status": "pass", "errors": [], "warnings": [] }, { "name": "Generic.Test.Artifact2", "path": "./artifacts/Artifact2.yaml", "status": "fail", "errors": [ "Generic.Test.Artifact2: query: Call to Artifact.Generic.Client.Info contains unknown parameter Foo", "Generic.Test.Artifact2: query: Unknown artifact reference Generic.Unknown.Artifact", "Generic.Test.Artifact2: query: Unknown plugin infoxxxx()" ], "warnings": [] }, { "name": "Generic.Test.Artifact3", "path": "./artifacts/Artifact3.yaml", "status": "warning", "errors": [], "warnings": [ "\u003cyellow\u003eSuggestion\u003c/\u003e: Add EXECVE to artifact's required_permissions or implied_permissions fields" ] }, { "name": "Unknown", "path": "./artifacts/Artifact4.yaml", "status": "fail", "errors": [ "yaml: unmarshal errors:\n line 7: field invalid not found in type proto.Artifact" ], "warnings": [] } ] }