Enhanced Input Sanitization #693

Merged
tobru merged 4 commits from docs/forgejo-config-injection-design into main 2026-09-23 08:38:53 +00:00
Owner
No description provided.
A Codey config value that contains a newline adds its own line to the
Forgejo config file in the pod. The tenant then picks the setting name.
A name ending in __FILE makes Forgejo read a file from the pod into
app.ini, so any org member who may create a Codey instance can read the
pod's service account token and mounted secrets.

Documents the verified chain (CRD -> AppCat -> chart -> init script ->
environment-to-ini), why _0X is not a __FILE bypass, and a two-layer
fix: a shared value validator on the CRD forms plus a hard check at the
Kubernetes write boundary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The live form_config exposes five free-text fields that write into
forgejoSettings.config, so the injection is reachable in production.
Landing Page takes a URL and needs the allow_equals flag.

Upstream reports and the blast-radius measurement are out of scope by
decision, recorded in the document.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(security): reject spec values that can forge a config line
All checks were successful
Tests / test (push) Successful in 1m44s
869923a9ba
A Codey config value with a newline opens a second line in the config
file the Forgejo chart renders, which lets the tenant pick the next
setting name. A name ending in __FILE makes Forgejo read a file from the
pod into app.ini, so any org member who may edit an instance could read
the pod's service account token and mounted secrets.

Two layers now guard it. Every generated form field rejects control
characters, "=" and a leading "[" in a value; a field may opt back into
"=" with the new form_config flag allow_equals, which the Landing Page
field needs. validate_spec_tree runs again in create_instance and
update_spec, right before the API call, so restore and staff paths are
covered even though they never touch a form.

FQDN entries must now be a lower-case hostname with at least three
labels, the rule the custom domain dialog states and only the browser
enforced. Array item patterns use re.fullmatch: "$" also matches before
a trailing newline, so re.match let exactly the injected character pass.

check_instance_specs scans existing instances for values that would now
be refused. Run it before rollout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(security): check only what a spec write adds
All checks were successful
Tests / test (push) Successful in 1m39s
78ea7c5bbb
Review findings on the config injection guard, before it ships:

- Every writer sends the whole live spec, and mariadbSettings is a
  multi-line my.cnf block by design. Checking the whole spec made such
  instances impossible to edit, pause, restore, or delete while
  protected. validate_spec_tree now takes the spec already in the
  cluster as a baseline and checks only values a write adds or changes.
  update_spec uses the live spec, a restore uses its source's spec.
- Dict keys are checked too, unless the baseline already has them: the
  chart renders key and value into the same line.
- FQDNs need two labels, not three. "No apex domain" is a rule of the
  restricted custom-domain dialog only; an unrestricted plane may serve
  an apex domain, and those instances could no longer be edited.
- display_name gets no guard. It never reaches a config file, and names
  like "[staging] Git" stay valid.

update_spec now reads the live spec on every call. Its three callers
have it cached already; the tests that never set one now state it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
tobru merged commit fc9143b8b7 into main 2026-09-23 08:38:53 +00:00
tobru deleted branch docs/forgejo-config-injection-design 2026-09-23 08:38:53 +00:00
Sign in to join this conversation.
No description provided.