Skip to content

fix(ssh): enforce firewall rules on cloud instances#6588

Merged
otavio merged 1 commit into
masterfrom
fix/cloud-firewall-enforcement
Jul 1, 2026
Merged

fix(ssh): enforce firewall rules on cloud instances#6588
otavio merged 1 commit into
masterfrom
fix/cloud-firewall-enforcement

Conversation

@geovannewashington

Copy link
Copy Markdown
Member

What

Run the SSH firewall check on cloud instances, not just pure enterprise
ones. Active "Deny" rules are now evaluated at connection time in cloud.

Why

Session.Evaluate gated checkFirewall behind IsEnterprise() && !IsCloud(), so cloud connections skipped it entirely and fell through to
the billing check. An operator could create an Active Deny rule, see it
listed in the UI, and still connect — the rule had no effect.

This is a silent enforcement bypass of an access-control feature. Firewall
rules are a documented Cloud + Enterprise capability (included on the free
Cloud tier), so they are expected to work in cloud. The root cause is the
recurring "cloud excludes enterprise" assumption, where it should be
"cloud extends enterprise": a cloud instance is also an enterprise
instance and should run the firewall check in addition to billing.

Changes

  • ssh/session/session.go: decouple the firewall check from the license
    check in Session.Evaluate and run it whenever IsEnterprise() or
    IsCloud() is set. License stays enterprise-only, billing stays
    cloud-only, firewall now spans both.
  • ssh/session/session_test.go: cover firewall enforcement across
    community, enterprise, and cloud modes to lock in the behaviour.

Testing

  1. Run a cloud instance
  2. Create an Active firewall rule: Action Deny, Any IP, All users,
    All devices.
  3. Open a web terminal (or SSH) to a device in that namespace.

Expected: the connection is denied by the rule. Before this change it
succeeded and the Deny rule was never evaluated.

Closes: shellhub-io/team#166

Session.Evaluate ran the firewall check only in pure enterprise mode, so
cloud connections skipped it entirely. Active Deny rules appeared in the
UI but were never evaluated at connection time, silently bypassing an
access-control feature that Cloud is documented to support.

The guard assumed cloud excludes enterprise, when cloud actually extends
it. Decouple the firewall check from the license check and run it
whenever either mode is enabled, so cloud enforces firewall rules
alongside billing instead of in place of it.
@geovannewashington geovannewashington self-assigned this Jul 1, 2026
@geovannewashington geovannewashington requested a review from a team as a code owner July 1, 2026 12:20
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished @geovannewashington's task in 6m 39s —— View job


Code Review Complete

Reviewed 2 files across code quality, security, testing, Go patterns, and architecture — no new issues found. The code looks good as-is.

Review highlights:

  • The fix correctly decouples checkFirewall from the license gate, using envs.IsEnterprise() || envs.IsCloud() to ensure firewall rules are enforced in both modes.
  • The check order (license → firewall → billing) is sound, with each check short-circuiting on failure.
  • Cross-repo verification confirms the cloud handler at cloud/internal/cloud/routes/firewall.go already serves the /internal/firewall/rules/evaluate endpoint for both modes — the SSH server just never called it in cloud mode before this PR.
  • Tests cover the core regression (cloud firewall bypass) and all three deployment modes (cloud, enterprise, community).

If you push additional changes and want a new review, tag @shellhub-io/admin and a team member can trigger it.

@otavio otavio merged commit a3c6c7b into master Jul 1, 2026
24 of 25 checks passed
@otavio otavio deleted the fix/cloud-firewall-enforcement branch July 1, 2026 13:35
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