Skip to content

Enforce tenant isolation for Ingress routes, snippets and ATS config - #382

Open
shukitchan wants to merge 6 commits into
apache:masterfrom
shukitchan:security-validation-highs
Open

shukitchan wants to merge 6 commits into
apache:masterfrom
shukitchan:security-validation-highs

Conversation

@shukitchan

Copy link
Copy Markdown
Contributor

This pull request introduces significant security improvements and configurability to the ATS Ingress Controller, particularly around the handling of ConfigMaps and Lua server snippets. The main focus is on hardening the dynamic configuration path to prevent privilege escalation and code injection, while providing operators a safe, auditable way to extend configuration as needed. Additionally, new tests and documentation have been added to support these changes.

ConfigMap RBAC and Allowlist Hardening:

  • Added a strict allowlist (builtinAllowedRecords) of records.config keys that can be set via ConfigMaps, blocking all others by default. The allowlist can be extended by operators using the CONFIGMAP_RECORD_ALLOWLIST environment variable in the pod spec. Only keys starting with proxy.config. are accepted, and entries can be exact or prefix matches. [1] [2]
  • Rejected config values containing whitespace or control characters to prevent injection or malformed configuration.
  • Updated RBAC: ClusterRole now only grants access to Endpoints (not ConfigMaps, Secrets, etc.), and a new namespaced Role/RoleBinding pair grants access to ConfigMaps only in the controller's namespace, following least-privilege principles. [1] [2] [3]

Lua Server Snippet Sandboxing:

  • Introduced a restricted execution environment for Lua server snippets, allowing only a safe subset of global functions and constants. Dangerous primitives (e.g., os, io, require, metatables, debug) are not accessible, and any attempt to use them results in a runtime error that is safely logged and contained. [1] [2]

Testing and Documentation:

  • Added comprehensive unit tests for both the ConfigMap allowlist logic and the Lua snippet sandbox, ensuring that only allowed keys/values are applied and that sandbox escapes are not possible. [1] [2]
  • Updated documentation to explain the new allowlist mechanism, its security rationale, and how operators can safely extend it. [1] [2]

Other Minor Improvements:

  • Added missing imports and minor code cleanups to support new features. [1] [2]

These changes collectively provide a much safer and more auditable configuration path for ATS Ingress, reducing the risk of privilege escalation or code execution via Kubernetes ConfigMaps or Ingress annotations.

shukitchan and others added 6 commits September 24, 2026 17:06
Reproduces the 2026-08-11 scan's HIGH/critical findings as failing Go tests
against the miniredis+FakeATS harness (no cluster needed). Each asserts the
secure invariant its candidate patch establishes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ConfigMap records now filtered against a safe allowlist; extra keys opt-in via
CONFIGMAP_RECORD_ALLOWLIST. Flips TestValidation_F002 green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First-writer-wins host ownership registry on IgHandler; cross-namespace claims
on an already-claimed host are rejected. Updates validation harness constructor
to the new keyed IgHandler literal. Flips TestValidation_F003 green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seeds new-side temp keys from the live route set (seedTempHostPath) so an
Ingress update can no longer replace co-owners' routes via SUNIONSTORE.
Resolved test-file overlap with bug_03 by keeping both patches' added tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes the unused cluster-wide secrets/services/namespaces/events read from
the chart ClusterRole; narrows what a stolen ServiceAccount token can reach
(amplifier for f001). No controller code path uses the Secrets API.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Run server snippets in an allowlisted sandbox: setfenv() each snippet into a
fresh env where only ts.*, string/table/math, basic builtins and TS_LUA_*
resolve; os/io/require/package/loadstring/dofile/debug resolve nil; writes
stay in the throwaway env. loadstring is nil-checked and the call is
pcall-wrapped so a hostile or broken snippet logs ts.error instead of
raising in the data plane.

Adds a regression test asserting every escape primitive (metatable ops,
_G, getfenv/setfenv, load/loadfile/dofile, coroutine, debug, package)
resolves nil inside a snippet. Validated on LuaJIT 2.1 (Lua 5.1): probe
test fails pre-patch, full busted suite 7/7 post-patch incl. legacy
Test - Snippet (ts.* snippets unaffected).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@shukitchan shukitchan self-assigned this Sep 30, 2026

This branch has not been deployed

No deployments
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