Skip to content

fix: consolidate contributor scan-history hardening - #153

Open
mldangelo-oai wants to merge 15 commits into
mainfrom
mdangelo/codex/contributor-scan-history-hardening
Open

fix: consolidate contributor scan-history hardening#153
mldangelo-oai wants to merge 15 commits into
mainfrom
mdangelo/codex/contributor-scan-history-hardening

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Summary

  • Preserve Gautam Sharma's original bounded scan-history matching contribution and its author attribution.
  • Preserve the original scan-history failure-containment, renderer resilience, and fixed-width severity-badge commits from mldangelo.
  • Reconcile bounded pagination with per-pair error containment so a failed comparison does not suppress healthy pairs on later history pages.

Verification

  • Full local TypeScript test suite: 481 passed, 6 expected platform/integration skips, 0 failures, 4361 assertions.
  • Focused history renderer and CLI workbench tests: 28 passed.
  • TypeScript compilation and formatting checks passed.

GautamSharma99 and others added 5 commits July 30, 2026 10:05
`scans match --all` aborted the whole campaign when one batch produced
conflicting confirmed and uncertain findings. The throw fires while
building `comparisons` for a batch, which is after every earlier batch
has already called `save-scan-comparison`, so the run exited nonzero with
partially persisted match state and no way to resume.

Contain each batch instead. A batch that cannot be matched is counted,
warned about, and skipped; the remaining batches still save. When nothing
matched at all the first failure is rethrown, so an unwritable workbench
or a rejected credential still surfaces as an error rather than a warning
attached to a successful no-op.

The conflict check itself is unchanged: refusing to persist conflicting
confirmed and uncertain findings is deliberate, and its existing test
passes unmodified.
runWorkbench only checks that the workbench response parsed as a JSON
object, so nothing validates the shape between the Python workbench and
the history renderer. The renderer then cast fields unconditionally, so a
plugin or database that drifted from the expected shape surfaced as an
unhandled TypeError with a stack trace instead of a usable view.

Confirmed crashes: a finding whose `severity` is absent or null threw on
`severity.level`; a `scans` row without `progress` threw on
`progress.status`; a `knownSince` that is not a parsable timestamp threw
RangeError out of Intl.DateTimeFormat.

Read every field through small optional accessors instead. Missing data
now renders as empty or `UNKNOWN`, an unparsable `knownSince` falls back
to its raw value, and an unrecognized severity sorts below the known
levels rather than above CRITICAL, where indexOf's -1 had placed it.

Output for well-formed payloads is unchanged; the existing renderer tests
assert exact text and pass unmodified.
`severity.padEnd(8)` is a no-op for INFORMATIONAL, which is thirteen
characters, so informational findings pushed their title five columns to
the right of every other severity and broke the table:

      HIGH      Short severity row
      INFORMATIONAL  Long severity row

The badge width and the two indents that line up beneath it were three
independent literals (8, 14, 14), so they only agreed while every label
happened to fit in eight columns.

Derive all of them from the label set and abbreviate INFORMATIONAL to
INFO. Both values are unchanged at 8 and 14, so only the informational
label differs; spelling the word out instead would cost every finding
title five characters, which matters most at the 48-column minimum.
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review Please review the exact current full head 1ac5da8, including the contributor-attributed implementation, focused regressions, platform behavior, package boundaries, and unresolved review findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ac5da8227

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdk/typescript/README.md
Comment thread sdk/typescript/src/cli.ts Outdated
Comment thread sdk/typescript/src/scan-history-renderer.ts Outdated
Comment thread sdk/typescript/src/cli.ts Outdated
Comment thread sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py Outdated
Comment thread sdk/typescript/src/scan-history-renderer.ts
Comment thread sdk/typescript/_bundled_plugin/scripts/workbench_scan_history.py Outdated
Comment thread sdk/typescript/src/scan-history-renderer.ts
mangeshraut712 and others added 4 commits July 30, 2026 11:06
informational is a valid findings severity and is exported to SARIF/CSV,
but report.md filtered it out via REPORTABLE_SEVERITIES, so all-
informational scans rendered as "No findings". Include informational in
the reportable set and cover it with a projection unit test.

Fixes #48
report.md filters findings to REPORTABLE_SEVERITIES, which excludes
informational. A scan whose findings are all informational therefore
sealed them into findings.json and exported them to SARIF and CSV while
the human-facing report stated that no findings survived the discovery,
validation and reportability gates. The three artifacts of one scan
contradicted each other, and the one a person reads was the one missing
data.

The exclusion itself is deliberate -- src/cli.ts draws the same line for
--fail-on-severity and keeps a separate DISPLAY_SEVERITIES that adds
informational -- so the report keeps its reportable-only body. It now
records how many findings were held back, with their severity mix, and
points at the artifacts that retain them.

references/final-report.md described the ordering as critical through
low without stating that informational is dropped, so it now says so.

Fixes #48
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head 8d6fa32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d6fa32fcf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdk/typescript/src/cli.ts Outdated
Comment thread sdk/typescript/tests-ts/api.test.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 8d6fa32fcf

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head b4d0ee3

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: b4d0ee3e2b

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4d0ee3e2b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdk/typescript/src/cli.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head 313768a

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 313768a97b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdk/typescript/src/cli.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 313768a97b

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 142cc8b

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 142cc8bb6f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sdk/typescript/src/cli.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 142cc8bb6f

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 3054e90

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 3054e90645

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 3054e90645

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

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.

4 participants