Skip to content

fix: dockerfile cleanup and https for IP lookup endpoints#1078

Open
memosr wants to merge 1 commit into
base:mainfrom
memosr:fix/dockerfile-cleanup-and-https-ip-lookup
Open

fix: dockerfile cleanup and https for IP lookup endpoints#1078
memosr wants to merge 1 commit into
base:mainfrom
memosr:fix/dockerfile-cleanup-and-https-ip-lookup

Conversation

@memosr
Copy link
Copy Markdown

@memosr memosr commented May 14, 2026

Summary

Two small but real fixes in different files:

  1. geth/Dockerfile — Fix broken apt cache cleanup that leaves package index files in the image
  2. op-node-entrypoint + base-consensus-entrypoint — Upgrade IP lookup URLs from http:// to https:// (security fix)

Fix 1 — Dockerfile apt cleanup

File: geth/Dockerfile (line 32)

- rm -rf /var/lib/apt/lists
+ rm -rf /var/lib/apt/lists/*

Without the glob, rm -rf cannot delete the directory itself (it's non-empty), leaving all apt package index files behind and bloating the resulting image.

The other client Dockerfiles in this repo already use the correct pattern:

  • reth/Dockerfile:30 — ✅ /var/lib/apt/lists/*
  • reth/Dockerfile:61 — ✅ /var/lib/apt/lists/*
  • nethermind/Dockerfile:38 — ✅ /var/lib/apt/lists/*

Only geth/Dockerfile was missing the glob.

Fix 2 — HTTPS for IP lookup (security)

Files: op-node-entrypoint (lines 7-10), base-consensus-entrypoint (lines 7-10)

- "http://ifconfig.me"
- "http://api.ipify.org"
- "http://ipecho.net/plain"
- "http://v4.ident.me"
+ "https://ifconfig.me"
+ "https://api.ipify.org"
+ "https://ipecho.net/plain"
+ "https://v4.ident.me"

Why this matters

The values returned from these endpoints are stored in $PUBLIC_IP and used directly as the node's advertised P2P address. Over plain HTTP, a network-level attacker positioned between the node and any of these services could:

  1. Intercept the response
  2. Inject a forged IP that passes the ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ regex check
  3. Cause the node to broadcast the wrong address to its peer network

All four services support HTTPS — there's no downside to making the switch.

Verification

  • ✅ All three changes verified locally
  • ✅ Pattern matches existing correct usage elsewhere in the repo
  • ✅ Target HTTPS endpoints all confirmed accessible

Two small but real fixes:

1. geth/Dockerfile (line 32) — Add missing /* to apt cache cleanup.
   Without the glob, the rm -rf cannot delete the directory (it's not
   empty), leaving all apt package index files behind and bloating the
   image. The reth and nethermind Dockerfiles in this repo already use
   the correct pattern (rm -rf /var/lib/apt/lists/*).

2. op-node-entrypoint and base-consensus-entrypoint — Convert 4 IP
   lookup URLs from http:// to https:// in both files.

   The returned IP is used directly as the node's advertised P2P address.
   Using HTTP means a network-level attacker could intercept the request
   and inject a forged IP, causing the node to broadcast a wrong address
   to the entire peer network. All four services (ifconfig.me, api.ipify.org,
   ipecho.net, v4.ident.me) support HTTPS.
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

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.

2 participants