Repository navigation
Conversation
Feat/export command
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fmt = export_mod.normalize_format(tokens[0]) | ||
| if fmt is not None: | ||
| tokens = tokens[1:] |
There was a problem hiding this comment.
/export @out/results.sarif calls normalize_format() on the path. That helper raises ExportError instead of returning None, so format inference is never reached. The call also sits outside the error handler, leaving the user with no export and no helpful status message. Use a non-raising format lookup here, then infer the format from the remaining path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/tui.py
Line: 5149-5151
Comment:
**Path-only export fails**
`/export @out/results.sarif` calls `normalize_format()` on the path. That helper raises `ExportError` instead of returning `None`, so format inference is never reached. The call also sits outside the error handler, leaving the user with no export and no helpful status message. Use a non-raising format lookup here, then infer the format from the remaining path.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| dest_path = Path(dest) | ||
| if dest_path.is_dir(): | ||
| raise ExportError(f"{dest_path} is a directory") |
There was a problem hiding this comment.
Plain /export passes the current directory to write_export(), which rejects directories. /export md and /export json ~/reports/ fail for the same reason. Append default_filename(fmt) when the destination is a directory so the advertised default and directory forms write a file.
| dest_path = Path(dest) | |
| if dest_path.is_dir(): | |
| raise ExportError(f"{dest_path} is a directory") | |
| dest_path = Path(dest) | |
| if dest_path.is_dir(): | |
| dest_path = dest_path / default_filename(fmt) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/export.py
Line: 261-263
Comment:
**Default export writes nothing**
Plain `/export` passes the current directory to `write_export()`, which rejects directories. `/export md` and `/export json ~/reports/` fail for the same reason. Append `default_filename(fmt)` when the destination is a directory so the advertised default and directory forms write a file.
```suggestion
dest_path = Path(dest)
if dest_path.is_dir():
dest_path = dest_path / default_filename(fmt)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| uri, _relative, line, _column = _location(finding, target_path) | ||
| location = f"{uri}:{line}" if line else uri | ||
|
|
||
| lines += [f"### {index}. {finding.title or category}", ""] |
There was a problem hiding this comment.
Markdown headings write finding.title without _scrub(). If a Hardcoded Secret finding includes a token in its title, such as ghp_ followed by 20 letters, the report preserves it unchanged. report_finding() stores the supplied title without removing tokens. Descriptions, snippets, and fixes use the protection that the heading skips. Scrub the title before adding it to the report.
How this was verified: report_finding() stores the supplied title unchanged, and the Markdown heading writes it directly even though redact() removes matching tokens.
| lines += [f"### {index}. {finding.title or category}", ""] | |
| lines += [f"### {index}. {_scrub(finding.title or category)}", ""] |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/export.py
Line: 195
Comment:
**Report headings expose tokens**
Markdown headings write `finding.title` without `_scrub()`. If a Hardcoded Secret finding includes a token in its title, such as `ghp_` followed by 20 letters, the report preserves it unchanged. `report_finding()` stores the supplied title without removing tokens. Descriptions, snippets, and fixes use the protection that the heading skips. Scrub the title before adding it to the report.
**How this was verified:** `report_finding()` stores the supplied title unchanged, and the Markdown heading writes it directly even though `redact()` removes matching tokens.
```suggestion
lines += [f"### {index}. {_scrub(finding.title or category)}", ""]
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if not path.is_absolute(): | ||
| return PurePosixPath(*path.parts).as_posix(), True |
There was a problem hiding this comment.
_artifact_uri() writes relative filenames into a URI without escaping them. A filename such as src/a#b.py becomes a reference to src/a with a fragment. A literal %20 in a filename can be read as a space. Reports for these files can point readers to the wrong source. Escape relative path components while preserving /, including the branch that makes absolute paths relative to the target.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/export.py
Line: 354-355
Comment:
**Report paths point elsewhere**
`_artifact_uri()` writes relative filenames into a URI without escaping them. A filename such as `src/a#b.py` becomes a reference to `src/a` with a fragment. A literal `%20` in a filename can be read as a space. Reports for these files can point readers to the wrong source. Escape relative path components while preserving `/`, including the branch that makes absolute paths relative to the target.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| fd, tmp_name = tempfile.mkstemp(dir=parent, prefix=f".{dest_path.name}.", suffix=".tmp") | ||
| tmp_path = Path(tmp_name) | ||
| try: |
There was a problem hiding this comment.
tempfile.mkstemp() runs outside the block that converts write errors into ExportError. If /export json /readonly/report.json targets an existing but unwritable directory, the directory check passes and temporary-file creation raises an uncaught OSError. The user gets no useful error in the status line. Catch temporary-file creation errors too, without trying to clean up a file that was never created.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/export.py
Line: 268-270
Comment:
**Write failures lack feedback**
`tempfile.mkstemp()` runs outside the block that converts write errors into `ExportError`. If `/export json /readonly/report.json` targets an existing but unwritable directory, the directory check passes and temporary-file creation raises an uncaught `OSError`. The user gets no useful error in the status line. Catch temporary-file creation errors too, without trying to clean up a file that was never created.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| base = hashlib.sha256(f"{finding.fingerprint()}::{finding.title}".encode()).hexdigest()[:32] | ||
| fingerprint = base | ||
| if fingerprint in seen: | ||
| fingerprint = hashlib.sha256(f"{base}::{finding.id}".encode()).hexdigest()[:32] |
There was a problem hiding this comment.
Findings with the same category, file, and title get fingerprints that depend on discovery order and a random finding.id. report_finding() allows two same-title findings at different lines. The first gets the shared base fingerprint; later findings get fingerprints that change on a fresh scan because their IDs change. Consumers that match by fingerprint can treat unchanged findings as new. Build fingerprints from stable location details instead of order and random IDs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/export.py
Line: 374-377
Comment:
**Repeated findings get new identities**
Findings with the same category, file, and title get fingerprints that depend on discovery order and a random `finding.id`. `report_finding()` allows two same-title findings at different lines. The first gets the shared base fingerprint; later findings get fingerprints that change on a fresh scan because their IDs change. Consumers that match by fingerprint can treat unchanged findings as new. Build fingerprints from stable location details instead of order and random IDs.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.1 participant
Summary
Adds
/export, a slash command that writes all loaded findings to a shareable file. Today findings can only be copied one at a time with/copy, and the only report is the raw session JSON. There's no way to get results into CI or GitHub code scanning.Works on a fresh scan or on a session reopened via
/sessions.What's included
github/codeql-action/upload-sarif) and opened in VS Code's SARIF viewer. Each category becomes a rule tagged with its CWE. Paths are relative to the scanned directory (%SRCROOT%), and fingerprints are unique per finding so alerts don't collapse together.redact()helper. PoC payloads are omitted from SARIF and Markdown. The JSON export keeps them (redacted), so it should be treated like the session report./exportalso appears in/help, command completion and the rotating tips.Design notes
AGENTS.md: this is a slash command inside the interactive agent, not a new CLI subcommand.openhack/export.pywith no TUI or prompt_toolkit imports, so it's tested directly.tui.pyonly gets a thin_cmd_exporthandler, the registry entry, a tip and the dispatch branch.parse_format()returnsNonefor a non-format word, so/export <path>can be told apart from/export md.write_export()accepts a directory and uses the default filename for the format.Testing
tests/test_export.pyadds 33 tests covering:path:lineparsing and fingerprint uniqueness.write_exportbehaviour.The
/export <path>and plain/exportcases are regression tests. Reverting theparse_formatand directory-handling changes makes 8 of them fail.Run on Linux with Python 3.12. CI covers 3.11–3.13.
Not in this PR
security-severityis a per-category default, not derived from each finding's CVSS score. That would be a reasonable follow-up.e) could call the same function later.