Skip to content

Upload corgea.yaml with third-party scan reports - #167

Merged
leenk7991 merged 3 commits into
mainfrom
leen/upload-repo-policy-yaml
Sep 6, 2026
Merged

Upload corgea.yaml with third-party scan reports#167
leenk7991 merged 3 commits into
mainfrom
leen/upload-repo-policy-yaml

Conversation

@leenk7991

Copy link
Copy Markdown
Member

Summary

Third-party corgea upload only sent files named in the scanner report, so Repo policies never ingested.

upload_scan now also uploads corgea.yaml / corgea.yml found under cwd (nested included). The walk uses the same ignore filters as the rest of the CLI, and still includes a policy file sitting on the project root even if gitignored.

Blast / full-zip already ships the tree. No fusion or doghouse changes.

Related

Test plan

  • cargo test --bin corgea corgea_policy_files
  • ./harness check
  • From cor-1172-manual-e2e, with corgea.yaml present:
    ../cli/target/release/corgea upload checkmarx-cli.json --project-name cor-1172-e2e-yaml-4 --wait
  • SourceFile for that scan includes corgea.yaml
  • policies_policy has a Repo row (company_id=1, name ILIKE %yaml%, source_type=Repo)
  • Homebrew /opt/homebrew/bin/corgea was not the binary used (still 1.10.0)
  • Did not reuse projects 4297 / 4298 / 4301

Made with Cursor

leenk7991 and others added 2 commits September 3, 2026 15:10
Report-only uploads never sent the policy file, so Fusion never created Repo policies from Checkmarx/Semgrep/SARIF/Coverity/Fortify ingest.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor 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.

No merge blockers on 0696a65 (0c6f63c plus an empty main merge).

upload_scan now appends cwd corgea.yaml / corgea.yml to the third-party file list, which is the actual ingest gap: report-only /code-upload never sent the policy file, so Fusion/doghouse never created Repo rows. Blast/full-zip is untouched and already ships the tree.

Checked against the call graph and the existing upload contract:

  • Only parse_scan and Fortify reach upload_scan. Extra paths go through the same exists()/code-upload → retry loop as report files, with string-exact dedup before the existing uploaded_paths set.
  • Discovery uses the same ignore::WalkBuilder + standard_filters(true) as the zip walker, force-includes a gitignored root file, and skips gitignored nested copies. That matches the documented nested layout (frontend/corgea.yaml, backend/corgea.yaml) and does not pick up hidden paths such as .continue/mcpServers/corgea.yaml.
  • Tests pin basename match, empty tree, gitignored nested skip, gitignored root keep, and merge append/dedup. They do not HTTP-mock upload_scan; the production change there is the two-line merge, which is enough.

parse_scan still exits on an empty path list before upload_scan runs. That is pre-existing (clean reports never created a scan) and is not a reason to hold this PR — changing it would start creating empty third-party scans.

Open in Web View Automation 

Sent by Cursor Automation: pr-flow

Comment thread src/scan.rs

@corgea-security corgea-security 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.

Automated review risk: 4/5.

Automatic policy discovery introduces a local file disclosure risk by following symlinks.

Critical or high-priority changes must be addressed.

Automatic approval was not submitted: automated review found critical or high-priority findings.

@corgea-security corgea-security added the dennis-reviewed Dennis completed an automated review label Sep 3, 2026
Comment thread src/scan.rs
}
}
}
found.sort();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nitpick: Normalize policy paths before deduplication

Deduplication compares raw strings, so an existing equivalent path such as ./corgea.yaml or an absolute path does not match discovered corgea.yaml. This can upload the same file twice. Normalize paths before comparison or deduplicate by resolved filesystem identity.

Proof or reproduction:

assert_eq!(merge_corgea_policy_files(vec!["./corgea.yaml".into()], root.path()), vec!["./corgea.yaml", "corgea.yaml"]);

@corgea-security corgea-security 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.

Automated review risk: 2/5.

No merge blockers found. Policy discovery is reasonably tested, but path deduplication misses equivalent path spellings.

No critical or high-priority changes were found.

@corgea-security corgea-security 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.

Approved by Dennis: high policy risk and automated risk 2/5.

@cursor cursor 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.

No merge blockers on ba88482.

The resolved symlink thread is actually fixed. find_corgea_policy_files now gates both the force-include of a gitignored root file and the WalkBuilder pass on is_regular_file (!is_symlink() && is_file()), so a corgea.yaml / corgea.yml that is a symlink is never added. ignore::WalkBuilder still has follow_links off by default, so a directory symlink cannot walk an outside tree into discovery either. find_corgea_policy_files_skips_symlink_outside_repo is the security repro and now asserts the empty list.

Upload of paths already named in the scanner report is unchanged and still uses exists() + Form::file. That is pre-existing for report files, not this auto-discovery path, and is not a reason to hold the PR.

Nothing else new on this commit beyond the regular-file check and that test. Prior notes (empty-report exit before upload_scan, helper-only tests, gitignored-root include) stay out of scope.

Tooling here cannot leave a GitHub approval (head-SHA pin). The review itself is merge.

Open in Web View Automation 

Sent by Cursor Automation: pr-flow

@leenk7991
leenk7991 merged commit 9f17483 into main Sep 6, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dennis-reviewed Dennis completed an automated review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants