Skip to content

[codex] Add Phoenix malware intel lookup - #1

Draft
franksec42 wants to merge 2 commits into
mainfrom
codex-malware-intel-phxintel
Draft

[codex] Add Phoenix malware intel lookup#1
franksec42 wants to merge 2 commits into
mainfrom
codex-malware-intel-phxintel

Conversation

@franksec42

Copy link
Copy Markdown

Summary

Adds Phoenix package intelligence as an optional malware-firewall finding source for bumblebee scans.

Changes

  • Adds --malware-intel-url, --malware-intel-key-env, timeout, and insecure-test flags.
  • Adds internal/threatintel client for Phoenix /api/v1/packages/intel and /internal/v1/packages/intel responses.
  • Emits existing record_type=finding records when remote package intelligence returns malware data.
  • Supports --malware-intel-url phxintel.security shorthand, expanding to https://phxintel.security/api/v1/packages/intel.
  • Documents Phoenix endpoint registration, heartbeat, package inventory reporting, and the boundary between generic NDJSON output and Phoenix endpoint inventory JSON.

Impact

Operators can run endpoint malware exposure scans against Phoenix package intelligence without shipping full package records by using --findings-only.

Validation

  • rtk go test ./... passed: 223 tests across 24 packages.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request integrates remote malware intelligence lookups backed by Phoenix package intelligence into the bumblebee scanner. It introduces new command-line flags, a dedicated client to query the Phoenix API, and comprehensive documentation and tests. Feedback suggests optimizing the scanner by checking for context cancellation or prior errors before making remote lookups, and ensuring the HTTP response body is fully drained in the client to allow TCP connection reuse.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +253 to +258
if cfg.ThreatIntel != nil {
matches, err := cfg.ThreatIntel.Lookup(ctx, r)
if err != nil {
setEmitErr(err)
return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If a terminal error has already occurred during the scan (e.g., a previous finding emission failed or another worker encountered an error), or if the context has been cancelled, we should avoid making any further remote threat intelligence lookup calls. Making unnecessary HTTP requests after a failure wastes network resources and can significantly delay the termination of the scan, especially on large projects.

We should check ctx.Err() and emitErr before calling cfg.ThreatIntel.Lookup.

		if cfg.ThreatIntel != nil {
			if ctx.Err() != nil {
				return
			}
			emitErrMu.Lock()
			hasErr := emitErr != nil
			emitErrMu.Unlock()
			if hasErr {
				return
			}
			matches, err := cfg.ThreatIntel.Lookup(ctx, r)
			if err != nil {
				setEmitErr(err)
				return
			}

Comment on lines +122 to +130
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxSnippet))
return nil, fmt.Errorf("malware intel: server returned %d: %s", resp.StatusCode, strings.TrimSpace(string(snippet)))
}

var decoded response
if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil {
return nil, fmt.Errorf("malware intel: decode response: %w", err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure that the underlying TCP connection can be reused by the HTTP client's keep-alive mechanism, the response body must be fully drained (read to EOF) before it is closed. If the body is not fully drained, the connection will be closed instead of being returned to the connection pool, which can lead to socket exhaustion and high latency when making many sequential requests.

We should use io.Copy(io.Discard, resp.Body) to drain the body in both the error and success paths.

	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
		snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxSnippet))
		_, _ = io.Copy(io.Discard, resp.Body)
		return nil, fmt.Errorf("malware intel: server returned %d: %s", resp.StatusCode, strings.TrimSpace(string(snippet)))
	}

	var decoded response
	if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil {
		return nil, fmt.Errorf("malware intel: decode response: %w", err)
	}
	_, _ = io.Copy(io.Discard, resp.Body)

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