Skip to content

fix(e5): distinguish proved cloud downloads from uploads - #891

Open
chrisknvidia wants to merge 25 commits into
mainfrom
feat/christopherk/e5-cloud-downloads
Open

chrisknvidia wants to merge 25 commits into
mainfrom
feat/christopherk/e5-cloud-downloads

Conversation

@chrisknvidia

@chrisknvidia chrisknvidia commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #889.

Literal cloud-to-local downloads currently produce E5 findings and fail --fail-on-findings. Suppress only individually proved AWS/GCS downloads through supported cp, non-deleting AWS sync, or gsutil rsync commands. Upload occurrences retain their original locations.

The proof requires bounded, fully observed context and literal operands. Only comments, blank lines, supported literal set flags and simple directory changes are accepted before a proved download. Unknown setup and earlier transfers retain E5. Relative destinations after directory changes, moves, deletion flags, streams, unknown options, dynamic operands and compound commands remain conservative. SDK detection, scoring and exit policy are unchanged. Current main's E1 command parsing is preserved.

Rebased onto e14c450b; current head bdc072d5. This includes upstream formatting and frozen-dependency updates; all five feature patches remain unchanged.

  • 18 real source/installed-wheel CLI and MCP scans passed, including client initialization, scan calls, report/strict-exit checks and clean MCP shutdown. All 138 package files and 157 dependency versions match between environments.
  • The four updated dependencies were installed from the frozen lock in both environments. Scoped tests: 621 passed; the same 12 known PowerShell assertions failed. Current helper/test provenance matches upstream logic, and these failures were previously reproduced on pristine main.
  • Full-repository Ruff lint and formatting pass. No cloud transfer was executed.

Independent RG, runtime, regular code and structured security delta reviews found no new actionable PR defect. Native typed review bots were unavailable. Full integration was not rerun for this dependency-only refresh; earlier integration had two Claude CLI settings failures reproduced on its unchanged base. No live model inference or customer production run was performed.

Fresh hosted checks are pending. The known upstream PowerShell assertions remain a validation limitation; no test or timeout was weakened. Re-review requires clean current-head checks.

@chrisknvidia
chrisknvidia marked this pull request as ready for review October 20, 2026 20:10
@chrisknvidia
chrisknvidia requested a review from rng1995 October 20, 2026 20:10
@chrisknvidia
chrisknvidia marked this pull request as draft October 20, 2026 20:17
@chrisknvidia
chrisknvidia marked this pull request as ready for review October 20, 2026 20:24
@chrisknvidia
chrisknvidia force-pushed the feat/christopherk/e5-cloud-downloads branch from 7b64de5 to fa24619 Compare October 21, 2026 03:16

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]

Value and readiness

The direction proof is conservative and preserves per-occurrence upload locations. Reviewed code is approvable, although the branch currently conflicts with main and needs reconciliation before merging.

Material findings

No required code changes found. The proof rejects compound context, runtime operands, lookup rebinding, streams, unknown flags and host-language commands. Relative destinations remain conservative after directory changes. No prior blocker threads.

PIC tradeoffs

Only simple AWS/GCS transfers qualify; the 4096-character complete-prefix requirement deliberately limits false-positive relief in long scripts. SDK detection is unchanged.

Verification and gaps

Reviewed exact head 8768a630dcd98ec3ae26db8593165b3cfd1152ca, the full diff, pinned source, description, tests, and all prior review/comment threads. Read the full diff and pinned supply-chain helpers used for logical commands, compound-context ownership and shell tokens. Tests exercise uploads, setup rebinding, streams, CLI strict gates and source locations. No checks are attached to this exact head. Cited hosted run 39537680751 succeeded at fa24619, which is older than this head and is not exact-head CI evidence. Static review only; no contributor code, tests, installs, or external services were executed.

@chrisknvidia
chrisknvidia force-pushed the feat/christopherk/e5-cloud-downloads branch 3 times, most recently from 23ef80e to d526dcb Compare November 2, 2026 19:27
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia force-pushed the feat/christopherk/e5-cloud-downloads branch from 2349fdc to bdc072d Compare November 2, 2026 20:07

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]

Value and readiness

Ready for maintainer review. The per-occurrence proof removes E5 false positives for literal cloud downloads while keeping upload evidence and locations. The prior marked approval had no code blocker; its merge-conflict limitation is resolved at this MERGEABLE head.

Material findings

No required changes found. static_patterns_data_exfiltration.py:190-284 requires complete bounded preceding context, a supported shell host, literal operands, recognized non-deleting flags and a local non-stream destination. Unknown setup, earlier transfers, runtime expansion, command rebinding/compound syntax, and relative destinations after directory changes retain E5. The occurrence loop at lines 664-671 leaves SDK patterns and upload locations intact. Reviewed all 395 added test lines, including strict CLI exits, nearest occurrence locations, lookup/setup changes, context/length boundaries and the shell helpers, streams and host languages. Existing E1 parsing is retained.

PIC tradeoffs

The complete-prefix 4,096-character bound and small preamble/option allowlists deliberately limit false-positive relief. SDK detection and scoring policy remain unchanged. No additional PIC decision identified.

Verification and gaps

Read the complete three-file diff, exact-head analyzer, referenced logical-command/token/compound-context helpers, all marked reviews and comments/threads, and added regression assertions. Exact-head CI run 41516369359: changes, lint, OpenCode TypeScript Tests, DCO and docker-smoke succeeded. test-unit reports 18,047 passed and 15 failed; all failed test IDs exactly match main e14c450b run 41513800480, in PowerShell/quote ownership. There is no additional failing test ID; the overall suite remains red and GitHub reports UNSTABLE. No required-check names are configured. Static review only; no contributor code, tests, imports, builds, setup, regex probes, scanner runs or cloud transfers were executed.

Reviewed head: bdc072d53b6cf66e1b05406f04ed0262ca8c17bd.

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.

E5 labels cloud-to-local downloads as cloud exfiltration

2 participants