Skip to content

docs(evaluation): correct the report-only instructions - #41

Open
joeymussalli wants to merge 1 commit into
trustabl:mainfrom
joeymussalli:docs/evaluation-report-only
Open

docs(evaluation): correct the report-only instructions#41
joeymussalli wants to merge 1 commit into
trustabl:mainfrom
joeymussalli:docs/evaluation-report-only

Conversation

@joeymussalli

Copy link
Copy Markdown

Step 1 of "Trialling it" says to run in report-only mode, and "Running it here" tells you how:

SEVERITY_THRESHOLD: high             # report-only: omit this

Omitting it does not produce a report-only run. It leaves SEVERITY_THRESHOLD at its default of none, which only disables this script's severity gate. The trustabl binary still exits 1 on any finding at medium or above, and the script honours that exit independently of either threshold.

So a reader following the documented evaluation procedure gets a failed pipeline on their very first trial run -- exactly the outcome step 1 exists to avoid -- and the note in the code fence tells them the one thing that will not help. The actual switch, || true on the invoking command, is documented in codepipeline/README.md and codecatalyst/README.md but not in the guide that tells you to use it.

The Gating table has the same gap from the other direction. It lists four controls without saying that two of them belong to the binary and two to this script, which invites the reading that SEVERITY_THRESHOLD sets the bar. It does not; it adds a second bar. SEVERITY_THRESHOLD: critical does not permit high findings.

  • Say plainly that report-only is not the default and cannot be reached by leaving thresholds unset, with the || true form for both platforms.
  • Warn to remove || true at step 4, since it also suppresses the failure raised when the scan did not complete.
  • Add a "Whose behaviour" column to the Gating table and explain that the thresholds are additional gates layered on the binary's default.
  • Fix the misleading code-fence comment and cross-reference Gating.

Documentation only; no code changes.

Step 1 of "Trialling it" says to run in report-only mode, and "Running it here"
tells you how:

    SEVERITY_THRESHOLD: high             # report-only: omit this

Omitting it does not produce a report-only run. It leaves SEVERITY_THRESHOLD at
its default of `none`, which only disables *this script's* severity gate. The
trustabl binary still exits 1 on any finding at medium or above, and the script
honours that exit independently of either threshold.

So a reader following the documented evaluation procedure gets a failed
pipeline on their very first trial run -- exactly the outcome step 1 exists to
avoid -- and the note in the code fence tells them the one thing that will not
help. The actual switch, `|| true` on the invoking command, is documented in
codepipeline/README.md and codecatalyst/README.md but not in the guide that
tells you to use it.

The Gating table has the same gap from the other direction. It lists four
controls without saying that two of them belong to the binary and two to this
script, which invites the reading that SEVERITY_THRESHOLD sets the bar. It does
not; it adds a second bar. `SEVERITY_THRESHOLD: critical` does not permit high
findings.

- Say plainly that report-only is not the default and cannot be reached by
  leaving thresholds unset, with the `|| true` form for both platforms.
- Warn to remove `|| true` at step 4, since it also suppresses the failure
  raised when the scan did not complete.
- Add a "Whose behaviour" column to the Gating table and explain that the
  thresholds are additional gates layered on the binary's default.
- Fix the misleading code-fence comment and cross-reference Gating.

Documentation only; no code changes.
@joeymussalli

Copy link
Copy Markdown
Author

Closing in favor of #36 (Paramjeet) on the report-only / omitted SEVERITY_THRESHOLD docs.

@joeymussalli

Copy link
Copy Markdown
Author

Reopened — closed earlier by mistake. Leaving this up per author request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant