From 72d8bb2fc8e6dc42e0adaf7c615e407529aa61af Mon Sep 17 00:00:00 2001 From: altanoruc Date: Wed, 16 Sep 2026 22:14:26 +0000 Subject: [PATCH 1/7] fix(ansible): close env-file and Alloy privilege gaps --- ansible/README.md | 2 +- ansible/galaxy/CHANGELOG.md | 14 + ansible/molecule/default/converge.yml | 8 +- .../molecule/default/files/decdn-node-stub | 20 +- ansible/molecule/default/verify.yml | 18 +- ansible/molecule/grafana-cloud/prepare.yml | 2 +- ansible/molecule/grafana-cloud/verify.yml | 10 +- ansible/molecule/slow-readiness/verify.yml | 13 + ansible/molecule/validation/converge.yml | 260 ++++++++++++++++-- ansible/roles/decdn_node/README.md | 14 +- .../roles/decdn_node/files/decdn.env.example | 7 +- ansible/roles/decdn_node/tasks/main.yml | 137 +++++++-- ansible/roles/grafana_alloy/README.md | 5 +- ansible/roles/grafana_alloy/defaults/main.yml | 5 +- .../files/grafana-alloy.env.example | 4 +- ansible/roles/grafana_alloy/tasks/main.yml | 35 ++- .../roles/grafana_alloy/tasks/preflight.yml | 32 ++- .../grafana_alloy/tasks/validate-paths.yml | 5 +- ansible/roles/grafana_alloy/vars/main.yml | 2 +- 19 files changed, 524 insertions(+), 69 deletions(-) diff --git a/ansible/README.md b/ansible/README.md index d39d512..ed8c99a 100644 --- a/ansible/README.md +++ b/ansible/README.md @@ -201,7 +201,7 @@ control machine): ```bash umask 077 -sudo install -m 600 -o root -g root grafana-alloy.env /etc/decdn/grafana-alloy.env +sudo install -m 600 -o root -g root grafana-alloy.env /etc/grafana-alloy.env ``` with `roles/grafana_alloy/files/grafana-alloy.env.example` as the template (remote-write diff --git a/ansible/galaxy/CHANGELOG.md b/ansible/galaxy/CHANGELOG.md index 53cf3fe..0fa22bc 100644 --- a/ansible/galaxy/CHANGELOG.md +++ b/ansible/galaxy/CHANGELOG.md @@ -29,6 +29,20 @@ collection adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) a missing port, a path/query/fragment and userinfo are rejected at deploy time. OTLP export is always compiled in; no `--features otlp` build is needed. +### Fixed + +- Grafana Alloy credentials now default to root-controlled + `/etc/grafana-alloy.env`; preflight also rejects a non-root-owned or writable + parent directory and a root service identity. Teardown requires the managed + stamp on the unit's first line, and destructive paths reject both `.` and `..` + segments. +- `decdn config validate` no longer bash-sources `/etc/decdn/decdn.env`. It + loads the role-supported one-line EnvironmentFile assignment forms with a + non-expanding host-side parser, so `$VAR` / `$(...)` in an inventory-rendered + (`to_json`) RPC URL stay literal for both the gate and the daemon. +- Manual dir-mode's read-only ELF probe runs under `--check` instead of being + skipped and feeding empty output into the architecture assertion. + ## [0.1.0] — unreleased Initial packaging of the public deCDN node roles as a distributable collection. diff --git a/ansible/molecule/default/converge.yml b/ansible/molecule/default/converge.yml index f40a916..b7eb69b 100644 --- a/ansible/molecule/default/converge.yml +++ b/ansible/molecule/default/converge.yml @@ -16,7 +16,10 @@ # (install.yml) instead of letting it degrade to a bare liveness check. Must # stay in sync with the version string printed by files/decdn-node-stub. decdn_node_version: "0.0.0-molecule-stub" - decdn_rpc_url: "https://rpc.example.invalid/" + # Shell metacharacters + a single quote are deliberate: the role's host-side + # EnvironmentFile parser must preserve the value literally without executing + # the command substitution (the stub + verify.yml check both properties). + decdn_rpc_url: "https://rpc.example.invalid/?token=$HOME$(touch /tmp/decdn-rpc-expanded)"e=o'brien" # The ACCEPTED side of the decdn_extra_env gate (#39): with an inventory # decdn_rpc_url the role authors the whole env file, so extra_env is rendered # rather than rejected. Nothing else in any scenario sets it alongside a @@ -26,6 +29,9 @@ # escape means the variable is silently absent at runtime. decdn_extra_env: DECDN_MOLECULE_EXTRA: 'a "quoted" \ value' + # Makes the stub compare the environment received by `config validate` + # against the exact literal above (not merely assert that no command ran). + DECDN_MOLECULE_ASSERT_RPC_LITERAL: "1" decdn_chain_id: 421614 decdn_region: "US" decdn_payment_pool_address: "0x1111111111111111111111111111111111111111" diff --git a/ansible/molecule/default/files/decdn-node-stub b/ansible/molecule/default/files/decdn-node-stub index 01e5ba2..a8b7344 100755 --- a/ansible/molecule/default/files/decdn-node-stub +++ b/ansible/molecule/default/files/decdn-node-stub @@ -17,7 +17,9 @@ can be exercised end-to-end without a published release or a live chain: scenario uses this to prove the role's advisory probe warns (not fails) on timeout while the unit stays `running`. * `config validate ...` - -> parse the rendered node.toml as TOML and exit 0. A stub has no + -> parse the rendered node.toml as TOML and exit 0. In the default + scenario it also checks the exact literal DECDN_RPC_URL passed + by the role's non-expanding EnvironmentFile loader. A stub has no schema, so this checks syntax only; `molecule/schema` is what covers key-level drift against the real upstream field list. * `key-gen ...` -> stand in for the CLI's wallet generator (exercised by the @@ -128,6 +130,22 @@ def config_validate(args): if not os.path.isfile(config): print(f"stub config validate: no such config file: {config}", file=sys.stderr) return 1 + if os.environ.get("DECDN_MOLECULE_ASSERT_RPC_LITERAL") == "1": + expected_rpc = ( + "https://rpc.example.invalid/?token=$HOME" + "$(touch /tmp/decdn-rpc-expanded)"e=o'brien" + ) + expected_extra = 'a "quoted" \\ value' + if ( + os.environ.get("DECDN_RPC_URL") != expected_rpc + or os.environ.get("DECDN_MOLECULE_EXTRA") != expected_extra + ): + print( + "stub config validate: an EnvironmentFile value was expanded or " + "decoded incorrectly", + file=sys.stderr, + ) + return 1 try: with open(config, "rb") as fh: tomllib.load(fh) diff --git a/ansible/molecule/default/verify.yml b/ansible/molecule/default/verify.yml index 3cecad3..815052a 100644 --- a/ansible/molecule/default/verify.yml +++ b/ansible/molecule/default/verify.yml @@ -79,9 +79,23 @@ decdn_extra_env value correctly. Got: {{ env_body }} vars: env_body: "{{ decdn_env_body.content | b64decode }}" - env_rpc_expected: 'DECDN_RPC_URL="https://rpc.example.invalid/"' + env_rpc_expected: >- + DECDN_RPC_URL="https://rpc.example.invalid/?token=$HOME$(touch /tmp/decdn-rpc-expanded)"e=o'brien" env_expected: 'DECDN_MOLECULE_EXTRA="a \"quoted\" \\ value"' + - name: Check whether sourcing the RPC URL executed its command substitution + ansible.builtin.stat: + path: /tmp/decdn-rpc-expanded + register: decdn_rpc_expansion_marker + + - name: Assert config validate did not expand the RPC URL as a shell + ansible.builtin.assert: + that: + - not decdn_rpc_expansion_marker.stat.exists + fail_msg: >- + config validate expanded a literal command substitution embedded in + decdn_rpc_url; the env file must be loaded without bash source. + - name: Compute the env file's current sha256 ansible.builtin.stat: path: "{{ decdn_etc }}/decdn.env" @@ -352,7 +366,7 @@ - /etc/alloy - /var/lib/alloy - /etc/systemd/system/alloy.service - - /etc/decdn/grafana-alloy.env + - /etc/grafana-alloy.env - name: Assert no Alloy artifacts exist while observability is disabled ansible.builtin.assert: diff --git a/ansible/molecule/grafana-cloud/prepare.yml b/ansible/molecule/grafana-cloud/prepare.yml index ecc65ce..9baf8c5 100644 --- a/ansible/molecule/grafana-cloud/prepare.yml +++ b/ansible/molecule/grafana-cloud/prepare.yml @@ -42,7 +42,7 @@ # non-empty). Written directly at 0600 because preflight refuses anything else. - name: Stage the Grafana Cloud credential fixture on the host ansible.builtin.copy: - dest: /etc/decdn/grafana-alloy.env + dest: /etc/grafana-alloy.env owner: root group: root mode: "0600" diff --git a/ansible/molecule/grafana-cloud/verify.yml b/ansible/molecule/grafana-cloud/verify.yml index 3b21cb3..a26e0df 100644 --- a/ansible/molecule/grafana-cloud/verify.yml +++ b/ansible/molecule/grafana-cloud/verify.yml @@ -12,7 +12,7 @@ register: ga_files loop: - {path: /etc/alloy/config.alloy, mode: "0644"} - - {path: /etc/decdn/grafana-alloy.env, mode: "0600"} + - {path: /etc/grafana-alloy.env, mode: "0600"} - {path: /etc/systemd/system/alloy.service, mode: "0644"} - name: Assert ownership/modes of the Alloy artifacts @@ -101,7 +101,7 @@ # --- Secret hygiene ------------------------------------------------------------- - name: Read the credential fixture back ansible.builtin.slurp: - src: /etc/decdn/grafana-alloy.env + src: /etc/grafana-alloy.env register: ga_env_b64 - name: Assert the env file carries every required key verbatim @@ -174,7 +174,7 @@ ansible.builtin.assert: that: - >- - 'EnvironmentFile=/etc/decdn/grafana-alloy.env' in ga_unit + 'EnvironmentFile=/etc/grafana-alloy.env' in ga_unit # Alloy defines no --config.expand-env and no gRPC admin listener; # either flag makes `alloy run` exit non-zero, i.e. a crash-loop. # Checked against the ExecStart flag lines ONLY — the unit's comments @@ -266,7 +266,7 @@ - /etc/alloy - /var/lib/alloy - - name: Stage a foreign Alloy installation (no managed-by marker) + - name: Stage a foreign Alloy installation (marker mentioned after line one) ansible.builtin.copy: dest: "{{ item.path }}" content: "{{ item.content }}" @@ -276,6 +276,8 @@ content: | [Unit] Description=Somebody else's Alloy + # This role used to write: + # MANAGED BY the grafana_alloy role — do not edit by hand. [Service] ExecStart=/bin/true - path: /etc/alloy/config.alloy diff --git a/ansible/molecule/slow-readiness/verify.yml b/ansible/molecule/slow-readiness/verify.yml index dababcc..b02a5dc 100644 --- a/ansible/molecule/slow-readiness/verify.yml +++ b/ansible/molecule/slow-readiness/verify.yml @@ -23,6 +23,19 @@ - name: Gather service facts ansible.builtin.service_facts: + - name: Read node.toml for the disabled-observability branch + ansible.builtin.slurp: + src: /etc/decdn/node.toml + register: node_toml_b64 + + - name: Assert disabled Grafana with no explicit endpoint omits OTLP entirely + ansible.builtin.assert: + that: + - "'otlp_endpoint' not in (node_toml_b64.content | b64decode)" + fail_msg: >- + node.toml rendered observability.otlp_endpoint even though both + decdn_grafana_cloud_enabled is false and decdn_otlp_endpoint is empty. + - name: Assert the deploy survived a metrics-probe timeout with the unit running ansible.builtin.assert: that: diff --git a/ansible/molecule/validation/converge.yml b/ansible/molecule/validation/converge.yml index 629fd22..1fffc18 100644 --- a/ansible/molecule/validation/converge.yml +++ b/ansible/molecule/validation/converge.yml @@ -119,6 +119,63 @@ decdn_cli_manual_bin_src: "{{ stub_bin }}" decdn_release_target_dir: "" + # A read-only `file -b` probe must still execute under --check. Before this + # regression guard it was skipped and the following ELF assert saw stdout="". + - name: "Case binary-check-mode — dir-mode format probe runs during dry-run" + vars: + _decdn_check_dir: >- + {{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-checkbin/target/release + block: + - name: Create the check-mode binary directory on the control machine + ansible.builtin.file: + path: "{{ _decdn_check_dir }}" + state: directory + mode: "0755" + delegate_to: localhost + become: false + + - name: Stage a valid ELF pair for the check-mode probe + ansible.builtin.copy: + src: /bin/true + dest: "{{ _decdn_check_dir }}/{{ item }}" + mode: "0755" + loop: [decdn-node, decdn] + delegate_to: localhost + become: false + + - name: Dry-run decdn_node in manual dir-mode + check_mode: true + block: + - name: Include decdn_node under check mode + ansible.builtin.include_role: + name: decdn_node + vars: + decdn_node_manual_bin_src: "" + decdn_cli_manual_bin_src: "" + decdn_release_target_dir: "{{ _decdn_check_dir }}" + decdn_node_target: >- + {{ 'aarch64-unknown-linux-gnu' + if ansible_architecture in ['aarch64', 'arm64'] + else 'x86_64-unknown-linux-gnu' }} + decdn_rpc_url: "" + rescue: + - name: Record binary check-mode success at the next expected gate + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['binary-check-mode'] }}" + when: ansible_failed_task.name is match('^Require an RPC endpoint') + always: + - name: Remove the check-mode binary fixture + ansible.builtin.file: + path: "{{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-checkbin" + state: absent + delegate_to: localhost + become: false + - name: Restore binary-source facts after the check-mode probe + ansible.builtin.set_fact: + decdn_node_manual_bin_src: "{{ stub_bin }}" + decdn_cli_manual_bin_src: "{{ stub_bin }}" + decdn_release_target_dir: "" + # --- Case: range (numeric but below the daemon's minimum) --------------------- - name: "Case range — event_poll_interval_ms below 250" block: @@ -386,6 +443,33 @@ decdn_rejected: "{{ decdn_rejected + ['otlp-newline'] }}" when: ansible_failed_task.name == 'Validate optional string knobs (user_agent, otlp_endpoint)' + - name: "Case env-control-rpc — inventory RPC URL contains a tab" + block: + - name: Run decdn_node with control data in the inventory RPC URL + ansible.builtin.include_role: + name: decdn_node + vars: + decdn_rpc_url: '{{ "https://rpc.example.invalid/" ~ "\t" ~ "secret" }}' + rescue: + - name: Record inventory RPC control-character rejection + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['env-control-rpc'] }}" + when: ansible_failed_task.name == 'Validate the inventory RPC URL is one-line EnvironmentFile data' + + - name: "Case env-control-extra — extra environment value contains a tab" + block: + - name: Run decdn_node with control data in decdn_extra_env + ansible.builtin.include_role: + name: decdn_node + vars: + decdn_extra_env: + DECDN_MOLECULE_BAD: '{{ "left" ~ "\t" ~ "right" }}' + rescue: + - name: Record extra environment control-character rejection + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['env-control-extra'] }}" + when: ansible_failed_task.name == 'Validate decdn_extra_env keys and values' + # --- Case: no RPC endpoint anywhere (#39) ------------------------------------- # decdn_rpc_url may now be empty — but ONLY when the operator has provisioned # /etc/decdn/decdn.env on the host. Nothing has staged that file yet at this @@ -533,15 +617,16 @@ # --- Cases: grafana_alloy preflight gates (#55) -------------------------------- # The opt-in Grafana Cloud role's OWN fail-loud gates get the same treatment. - # All seven broken configs abort inside preflight-imported tasks BEFORE any + # Broken configs abort inside preflight-imported tasks BEFORE any # host mutation (user/package/config), so they cannot interfere with the other - # cases or start services. Tag deduping at the bottom handles two cases per - # gate. Fixtures are staged/restored per case; none of this touches decdn.env. - - name: "Case ga-no-secret — enabled without /etc/decdn/grafana-alloy.env" + # cases or start services. Repeated tags are preserved in the final multiset, + # so every sibling case must fail independently. Fixtures are staged/restored + # per case; none of this touches decdn.env. + - name: "Case ga-no-secret — enabled without /etc/grafana-alloy.env" block: - name: Stage only a placeholder credential path (absent file) ansible.builtin.file: - path: /etc/decdn/grafana-alloy.env + path: /etc/grafana-alloy.env state: absent - name: Run grafana_alloy with its credential file absent @@ -572,7 +657,7 @@ - name: Point the credential path at the out-of-tree symlink target ansible.builtin.file: src: /var/lib/grafana-alloy-fixture.env - dest: /etc/decdn/grafana-alloy.env + dest: /etc/grafana-alloy.env state: link force: true @@ -592,14 +677,14 @@ path: "{{ item }}" state: absent loop: - - /etc/decdn/grafana-alloy.env + - /etc/grafana-alloy.env - /var/lib/grafana-alloy-fixture.env - name: "Case ga-missing-key — GC_API_TOKEN line dropped" block: - name: Stage an otherwise-complete credential file without the token ansible.builtin.copy: - dest: /etc/decdn/grafana-alloy.env + dest: /etc/grafana-alloy.env owner: root group: root mode: "0600" @@ -621,14 +706,14 @@ always: - name: Remove the partial credential fixture ansible.builtin.file: - path: /etc/decdn/grafana-alloy.env + path: /etc/grafana-alloy.env state: absent - name: "Case ga-http-url — cleartext remote-write endpoint" block: - name: Stage a credential file with an http:// push URL ansible.builtin.copy: - dest: /etc/decdn/grafana-alloy.env + dest: /etc/grafana-alloy.env owner: root group: root mode: "0600" @@ -651,7 +736,7 @@ always: - name: Remove the http-url credential fixture ansible.builtin.file: - path: /etc/decdn/grafana-alloy.env + path: /etc/grafana-alloy.env state: absent - name: "Case ga-ratio — trace sampler keep-ratio above 1" @@ -668,6 +753,89 @@ decdn_rejected: "{{ decdn_rejected + ['pipeline-knobs'] }}" when: ansible_failed_task.name is match('^Validate the pipeline knobs') + - name: "Case ga-root-user — Alloy cannot run as root" + block: + - name: Run disabled grafana_alloy with root as its service user + ansible.builtin.include_role: + name: grafana_alloy + vars: + # The disabled path can delete a recognized installation's account. + decdn_grafana_cloud_enabled: false + grafana_alloy_user: root + rescue: + - name: Record ga-root-user rejection (only if the identity gate failed) + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['alloy-root-user'] }}" + when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') + + - name: "Case ga-root-group — Alloy cannot run with root group" + block: + - name: Run disabled grafana_alloy with numeric root as its service group + ansible.builtin.include_role: + name: grafana_alloy + vars: + decdn_grafana_cloud_enabled: false + grafana_alloy_group: "0" + rescue: + - name: Record ga-root-group rejection (only if the identity gate failed) + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['alloy-root-group'] }}" + when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') + + - name: "Case ga-root-user-alias — named account resolves to UID 0" + block: + - name: Stage a root-UID alias account + ansible.builtin.user: + name: alloy-root-alias + uid: 0 + group: root + non_unique: true + create_home: false + shell: /usr/sbin/nologin + + - name: Run disabled grafana_alloy with the root-UID alias + ansible.builtin.include_role: + name: grafana_alloy + vars: + decdn_grafana_cloud_enabled: false + grafana_alloy_user: alloy-root-alias + rescue: + - name: Record root-UID alias rejection + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['alloy-root-user-alias'] }}" + when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') + always: + - name: Remove the root-UID alias account + ansible.builtin.user: + name: alloy-root-alias + state: absent + remove: false + + - name: "Case ga-root-group-alias — named group resolves to GID 0" + block: + - name: Stage a root-GID alias group + ansible.builtin.group: + name: alloy-root-group-alias + gid: 0 + non_unique: true + + - name: Run disabled grafana_alloy with the root-GID alias + ansible.builtin.include_role: + name: grafana_alloy + vars: + decdn_grafana_cloud_enabled: false + grafana_alloy_group: alloy-root-group-alias + rescue: + - name: Record root-GID alias rejection + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['alloy-root-group-alias'] }}" + when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') + always: + - name: Remove the root-GID alias group + ansible.builtin.group: + name: alloy-root-group-alias + state: absent + - name: "Case ga-path-traversal — state directory escaping /var/lib" block: - name: Run grafana_alloy with a traversing state directory @@ -684,6 +852,60 @@ decdn_rejected: "{{ decdn_rejected + ['managed-paths'] }}" when: ansible_failed_task.name is match('^Constrain the managed Alloy paths') + - name: "Case ga-dot-segment — disabled teardown path contains a normalising dot" + block: + - name: Run disabled grafana_alloy with a dot segment in its state directory + ansible.builtin.include_role: + name: grafana_alloy + vars: + decdn_grafana_cloud_enabled: false + grafana_alloy_state_dir: /var/lib/alloy/. + rescue: + - name: Record ga-dot-segment rejection (only if the path gate failed) + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['managed-paths'] }}" + when: ansible_failed_task.name is match('^Constrain the managed Alloy paths') + + - name: "Case ga-nested-secret-path — credentials are not directly under /etc" + block: + - name: Stage a root-controlled nested credential directory + ansible.builtin.file: + path: /etc/alloy-credentials + state: directory + owner: root + group: root + mode: "0750" + + - name: Stage a root-owned credential file inside the nested directory + ansible.builtin.copy: + dest: /etc/alloy-credentials/grafana-alloy.env + owner: root + group: root + mode: "0600" + content: | + GC_PROM_REMOTE_WRITE_URL=https://prometheus-prod-xx.molecule.invalid/api/prom/push + GC_OTLP_ENDPOINT=https://otlp-gateway-prod-xx.molecule.invalid/otlp + GC_PROM_USERNAME=999888777 + GC_API_TOKEN=molecule-test-token + + - name: Run grafana_alloy against a nested credential path + ansible.builtin.include_role: + name: grafana_alloy + vars: + decdn_grafana_cloud_enabled: true + grafana_alloy_secret_file: /etc/alloy-credentials/grafana-alloy.env + rescue: + - name: Record nested secret-path rejection (only if its gate failed) + ansible.builtin.set_fact: + decdn_rejected: "{{ decdn_rejected + ['secret-path-nested'] }}" + when: ansible_failed_task.name is match('^Assert the secret file parent') + always: + - name: Remove the nested credential directory fixture + ansible.builtin.file: + path: /etc/alloy-credentials + state: absent + + - name: "Case ga-config-outside-dir — rendered config outside the managed directory" block: - name: Run grafana_alloy with a config file outside its config directory @@ -702,9 +924,9 @@ - name: Confirm every bad value was rejected by the role's own validation ansible.builtin.assert: that: - # unique(): the grafana_alloy cases append the SAME tag for different - # inputs guarded by one shared gate (both bad file shapes -> 'secret-file'). - - decdn_rejected | unique | sort == _decdn_expected | sort + # Keep duplicates: every case must contribute its own rejection. Deduping + # lets one broken sibling hide behind another case that uses the same gate. + - decdn_rejected | sort == _decdn_expected | sort fail_msg: >- Neither decdn_node nor grafana_alloy rejected every bad value at their validation asserts. @@ -713,8 +935,12 @@ the daemon/crash-loop it at config load, or ship credentials insecurely). vars: _decdn_expected: - ["cross-field", "binary-format", "range", "bounded-range", "shape", "bool", + ["cross-field", "binary-format", "binary-check-mode", "range", "bounded-range", "shape", "bool", "enum", "dict", "float", "list", "string", "string-newline", "otlp-https", "otlp-noport", "otlp-path", "otlp-userinfo", "otlp-colon", "otlp-port", - "otlp-newline", "env-gate", "env-extra", "env-content", "config-gate", - "secret-file", "secret-content", "pipeline-knobs", "managed-paths"] + "otlp-newline", "env-control-rpc", "env-control-extra", "env-gate", + "env-extra", "env-content", "config-gate", + "secret-file", "secret-file", "secret-path-nested", "secret-content", + "secret-content", "pipeline-knobs", "alloy-root-user", "alloy-root-group", + "alloy-root-user-alias", "alloy-root-group-alias", + "managed-paths", "managed-paths", "managed-paths"] diff --git a/ansible/roles/decdn_node/README.md b/ansible/roles/decdn_node/README.md index 5406825..6bf4017 100644 --- a/ansible/roles/decdn_node/README.md +++ b/ansible/roles/decdn_node/README.md @@ -302,11 +302,15 @@ sudo chmod 600 /etc/decdn/decdn.env # belt-and-braces: sudo may apply its own systemd's `EnvironmentFile` parser is shell-*like* but not a shell: one `KEY=value` per line, no `export`, no expansion — and it **skips** a line it cannot parse, logging a warning to `journalctl -u decdn-node` that is easy to miss, so the -variable is simply absent at runtime. Single-quote any value containing a space, -quote, backslash, `$` or backtick. (`decdn config validate` sources this file with -bash, which *would* expand `$VAR` and execute `$(…)`; quoting avoids both.) On the -inventory path the role applies that escaping to `decdn_extra_env` values for you — -but **not** to `decdn_rpc_url` itself, which is rendered raw. +variable is simply absent at runtime. Single-quote any host-provisioned value +containing a space, quote, backslash, `$` or backtick. On the inventory path the +role renders `decdn_rpc_url` and every `decdn_extra_env` value with `to_json` +(systemd EnvironmentFile double-quotes). `decdn config validate` loads the +role-supported one-line assignment forms with a non-expanding host-side parser +rather than bash `source`, so `$VAR` / `$(…)` in an inventory URL stay literal — +matching what the daemon receives. Host-provisioned values may be unquoted, +completely single-quoted, or JSON-style double-quoted; shell concatenation and +multiline EnvironmentFile forms are deliberately rejected by the deploy gate. ### The provenance record diff --git a/ansible/roles/decdn_node/files/decdn.env.example b/ansible/roles/decdn_node/files/decdn.env.example index 135645c..7093fa8 100644 --- a/ansible/roles/decdn_node/files/decdn.env.example +++ b/ansible/roles/decdn_node/files/decdn.env.example @@ -23,9 +23,10 @@ # expansion, no `export`, one KEY=value per line. It SKIPS a line it cannot parse # (logging a warning to `journalctl -u decdn-node` that is easy to miss), so the # variable is simply absent at runtime — surfacing much later as an auth failure. -# Single-quote any value containing a space, quote, backslash, `$` or backtick. -# The role's `decdn config validate` step sources this file with bash, which — -# unlike systemd — WOULD expand `$VAR` and execute `$(...)`; quoting avoids both. +# For the role's deploy-time validation, keep each assignment on one line and use +# either an unquoted value, one complete single-quoted value, or a JSON-style +# double-quoted value. Shell-style concatenation and multiline values are rejected. +# The parser does not use bash `source`, so `$VAR` / `$(...)` stay literal. # REQUIRED. The chain RPC endpoint — may embed a provider API key, which is why # this file exists at all. Public endpoints work for light use; run your own or use diff --git a/ansible/roles/decdn_node/tasks/main.yml b/ansible/roles/decdn_node/tasks/main.yml index a6bde2d..7da0765 100644 --- a/ansible/roles/decdn_node/tasks/main.yml +++ b/ansible/roles/decdn_node/tasks/main.yml @@ -164,6 +164,7 @@ - "{{ item }}" register: _decdn_bin_file changed_when: false + check_mode: false loop: - "{{ decdn_node_manual_bin_src }}" - "{{ decdn_cli_manual_bin_src }}" @@ -1068,12 +1069,13 @@ ansible.builtin.assert: that: - item.key is match('^[A-Za-z_][A-Za-z0-9_]*$') - - item.value | string is match('^[^\r\n]*$') + - item.value | string is match('^[^\x00-\x1f\x7f]*\Z') - item.key != 'DECDN_RPC_URL' fail_msg: >- decdn_extra_env keys must be POSIX environment-variable names and values must - contain no newline (systemd silently DROPS an unparseable EnvironmentFile - line, so the variable would be absent at runtime rather than wrong). + contain no ASCII control characters (systemd silently DROPS an unparseable + EnvironmentFile line, so the variable would be absent at runtime rather than + wrong). DECDN_RPC_URL is set from decdn_rpc_url and must not be overridden here. Offending entry: {{ item.key }}. loop: "{{ decdn_extra_env | default({}, true) | dict2items }}" @@ -1081,6 +1083,17 @@ label: "{{ item.key }}" no_log: true # values are secrets by design +- name: Validate the inventory RPC URL is one-line EnvironmentFile data + ansible.builtin.assert: + that: + - decdn_rpc_url | string is match('^[^\x00-\x1f\x7f]*\Z') + fail_msg: >- + decdn_rpc_url must contain no ASCII control characters; the role renders it + as one EnvironmentFile assignment and must preserve one value identically for + both systemd and the deploy-time config validation. + no_log: true + when: decdn_rpc_url | length > 0 + # --- User & directories ------------------------------------------------------- - name: Create decdn system group ansible.builtin.group: @@ -1293,8 +1306,9 @@ # to_json emits exactly the double-quoted, C-escaped form systemd's # EnvironmentFile parser expects, and ensure_ascii=false keeps UTF-8 literal # (systemd understands \xNN, not \uNNNN). It supplies the quotes itself, so a - # value with a space, '#', quote or backslash survives intact — the RPC URL - # included, which used to be interpolated raw. + # value with a space, '#', quote, `$` or backslash survives intact — the RPC + # URL included. The validate task below must NOT bash-source this file: + # bash would expand $VAR / $(...) inside those double quotes; systemd will not. content: | DECDN_RPC_URL={{ decdn_rpc_url | string | to_json(ensure_ascii=false) }} {% for k, v in (decdn_extra_env | default({}, true)) | dictsort %} @@ -1435,28 +1449,107 @@ # Runs as decdn_user because it reads the 0600 keystore password; DECDN_RPC_URL is # supplied from the same value systemd will hand the daemon. Skipped in check mode # (node.toml has not been written yet) — the asserts above are the check-mode gate. -# The RPC URL is sourced from the 0600 env file rather than passed via Ansible's +# The RPC URL is loaded from the 0600 env file rather than passed via Ansible's # `environment:`. `environment:` is implemented as a shell prefix on the module # command (`ENV=val /usr/bin/python3 ...`), so it lands in the target host's # process table and any local user can read the embedded API key out of `ps` for # the duration of the task — `no_log` hides it from Ansible's output, not from -# /proc. Sourcing the file systemd already reads keeps the secret on disk at 0600. +# /proc. A host-side, non-expanding parser reads the same file and passes the +# resulting environment directly to the CLI subprocess, keeping the secret out of +# the shell command line and preventing shell expansion. - name: Validate the rendered node.toml with the installed binary ansible.builtin.shell: executable: /bin/bash cmd: | set -euo pipefail - # Exit 3 is reserved for "the env file could not be sourced" so the report - # task below does not blame node.toml for a quoting typo in decdn.env. NOTE - # this is bash, not systemd's parser: it expands $VAR and executes $(...) - # and backticks, which systemd would pass through literally. Operators are - # told to single-quote values (files/decdn.env.example). - set -a - . {{ decdn_env_file | quote }} || { echo "env-file could not be sourced" >&2; exit 3; } - set +a - {{ decdn_cli_bin | quote }} config validate \ - --config {{ decdn_config_file | quote }} \ - --keystore-password-file {{ decdn_keystore_password_file | quote }} + # Exit 3 is reserved for "the env file could not be parsed" so the report + # task below does not blame node.toml for a quoting typo in decdn.env. + # Do NOT bash-source the file: bash expands $VAR and executes $(...) / + # backticks inside double quotes, which systemd's EnvironmentFile parser + # (and therefore the running daemon) would pass through literally. + python3 - \ + {{ decdn_env_file | quote }} \ + {{ decdn_cli_bin | quote }} \ + {{ decdn_config_file | quote }} \ + {{ decdn_keystore_password_file | quote }} \ + <<'PY' + import os, re, subprocess, sys + + def parse_double_quoted(value): + """Parse systemd's one-line double-quoted EnvironmentFile subset.""" + body = value[1:-1] + parsed = [] + index = 0 + while index < len(body): + char = body[index] + if char == '"': + raise ValueError("adjacent double-quoted segments are unsupported") + if char != "\\": + parsed.append(char) + index += 1 + continue + if index + 1 >= len(body): + raise ValueError("trailing backslash") + following = body[index + 1] + if following in ['"', "\\", "$", "`"]: + parsed.append(following) + else: + # In systemd double quotes, a backslash before any other + # character is preserved literally. + parsed.extend(["\\", following]) + index += 2 + return "".join(parsed) + + def validate_value(value): + """Reject bytes/code points systemd refuses plus unsupported controls.""" + for char in value: + codepoint = ord(char) + if codepoint < 0x20 or codepoint == 0x7F: + raise ValueError("ASCII control character") + if codepoint == 0xFEFF or 0xFDD0 <= codepoint <= 0xFDEF: + raise ValueError("Unicode noncharacter/BOM") + if (codepoint & 0xFFFF) in [0xFFFE, 0xFFFF]: + raise ValueError("Unicode noncharacter") + + try: + path, cli, config, pwfile = sys.argv[1:5] + except ValueError: + sys.stderr.write("env-file could not be parsed\n") + sys.exit(3) + env = os.environ.copy() + key_re = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + try: + with open(path, "r", encoding="utf-8") as fh: + for raw in fh: + line = raw.rstrip("\r\n") + if not line.strip() or line.lstrip().startswith("#"): + continue + if "=" not in line: + raise ValueError("missing assignment") + key, value = line.split("=", 1) + if not key_re.fullmatch(key): + raise ValueError("bad key") + if len(value) >= 2 and value[0] == value[-1] == '"': + value = parse_double_quoted(value) + elif len(value) >= 2 and value[0] == value[-1] == "'": + value = value[1:-1] + if "'" in value: + raise ValueError("single-quote concatenation is unsupported") + elif value[:1] in ['"', "'"] or value[-1:] in ['"', "'"]: + raise ValueError("unbalanced quote") + elif value and not re.fullmatch(r'''[^\s\\'"]+''', value): + raise ValueError("unsupported unquoted value") + validate_value(value) + env[key] = value + except Exception: + sys.stderr.write("env-file could not be parsed\n") + sys.exit(3) + result = subprocess.run( + [cli, "config", "validate", "--config", config, "--keystore-password-file", pwfile], + env=env, + ) + sys.exit(result.returncode) + PY become_user: "{{ decdn_user }}" become: true register: decdn_config_validate @@ -1476,11 +1569,11 @@ ansible.builtin.fail: msg: >- {% if decdn_config_validate.rc | default(0) == 3 %} - {{ decdn_env_file }} could not be sourced, so the config was never + {{ decdn_env_file }} could not be parsed, so the config was never validated: {{ decdn_config_validate.stderr | default('(no output)', true) }}. - This is a shell-syntax fault in the ENV FILE, not a problem with - {{ decdn_config_file }} — most often an unbalanced quote, or an unquoted - value containing a space. Single-quote the value and re-run. + This is a fault in the ENV FILE, not a problem with + {{ decdn_config_file }} — most often an unbalanced quote. Fix the + EnvironmentFile syntax and re-run. {% elif decdn_config_validate.rc is not defined %} `decdn config validate` could not be RUN ({{ decdn_config_validate.msg | default('no rc and no msg — re-run with -vvv', true) }}). diff --git a/ansible/roles/grafana_alloy/README.md b/ansible/roles/grafana_alloy/README.md index 9d718d4..37858de 100644 --- a/ansible/roles/grafana_alloy/README.md +++ b/ansible/roles/grafana_alloy/README.md @@ -28,10 +28,13 @@ machine): ```bash umask 077 -sudo install -m 600 -o root -g root grafana-alloy.env /etc/decdn/grafana-alloy.env +sudo install -m 600 -o root -g root grafana-alloy.env /etc/grafana-alloy.env ``` using `roles/grafana_alloy/files/grafana-alloy.env.example` as the template. The +credential path is intentionally restricted to a **direct child of `/etc`**. A +nested override (including the old `/etc/decdn/grafana-alloy.env`) is rejected: +write access to any ancestor is enough to replace a root-owned `0600` file. four required keys are `GC_PROM_REMOTE_WRITE_URL`, `GC_OTLP_ENDPOINT`, `GC_PROM_USERNAME` and `GC_API_TOKEN`; both URLs must be `https://`. `config.alloy` reads each of them at load time with `sys.env("GC_…")` — Alloy has diff --git a/ansible/roles/grafana_alloy/defaults/main.yml b/ansible/roles/grafana_alloy/defaults/main.yml index 2d8fd71..56232fa 100644 --- a/ansible/roles/grafana_alloy/defaults/main.yml +++ b/ansible/roles/grafana_alloy/defaults/main.yml @@ -33,8 +33,9 @@ grafana_alloy_state_dir: /var/lib/alloy # WAL/storage state, owned by the a # pairs (template: files/grafana-alloy.env.example). Root-owned 0600 — systemd # reads EnvironmentFile= as root BEFORE dropping to the agent user, so `alloy` # never needs read access to it and no secret transits this repo. Provision it on -# the target host like /etc/decdn/decdn.env. -grafana_alloy_secret_file: /etc/decdn/grafana-alloy.env +# the target host at this root-owned path. Keep it outside service-owned trees: +# write access to the parent is enough to unlink and replace a 0600 file. +grafana_alloy_secret_file: /etc/grafana-alloy.env # --- Network (loopback-only; AGENTS.md hard rule 2) --------------------------- # OTLP receivers bound to loopback ONLY — the daemon accepts telemetry pushed diff --git a/ansible/roles/grafana_alloy/files/grafana-alloy.env.example b/ansible/roles/grafana_alloy/files/grafana-alloy.env.example index 7c0d7cc..7bea71f 100644 --- a/ansible/roles/grafana_alloy/files/grafana-alloy.env.example +++ b/ansible/roles/grafana_alloy/files/grafana-alloy.env.example @@ -1,10 +1,10 @@ -# Template for /etc/decdn/grafana-alloy.env (root-owned 0600). +# Template for /etc/grafana-alloy.env (root-owned 0600). # # Provision it ON THE TARGET HOST — these values are your Grafana Cloud # credentials and must never be committed or carried through inventory: # # umask 077 -# sudo install -m 600 -o root -g root grafana-alloy.env /etc/decdn/grafana-alloy.env +# sudo install -m 600 -o root -g root grafana-alloy.env /etc/grafana-alloy.env # # Values come from your Grafana Cloud org: observe -> Application Observability -> # "Instructions"/details page shows the exact remote-write URL, OTLP endpoint, diff --git a/ansible/roles/grafana_alloy/tasks/main.yml b/ansible/roles/grafana_alloy/tasks/main.yml index f6751f6..a350962 100644 --- a/ansible/roles/grafana_alloy/tasks/main.yml +++ b/ansible/roles/grafana_alloy/tasks/main.yml @@ -17,6 +17,38 @@ string — it gates this entire role and the otlp_endpoint injection in roles/decdn_node/templates/node.toml.j2. +# Global, not preflight-only: the disabled path can delete the configured account +# after recognizing a managed unit, so root/0 must be refused before EITHER path. +- name: Resolve an existing Alloy service user + ansible.builtin.getent: + database: passwd + key: "{{ grafana_alloy_user }}" + fail_key: false + changed_when: false + +- name: Resolve an existing Alloy service group + ansible.builtin.getent: + database: group + key: "{{ grafana_alloy_group }}" + fail_key: false + changed_when: false + +- name: Require an unprivileged Alloy identity + ansible.builtin.assert: + that: + - (grafana_alloy_user | string) not in ['root', '0'] + - (grafana_alloy_group | string) not in ['root', '0'] + - >- + (ansible_facts.getent_passwd | default({})).get(grafana_alloy_user) is none + or (ansible_facts.getent_passwd[grafana_alloy_user][1] | int) != 0 + - >- + (ansible_facts.getent_group | default({})).get(grafana_alloy_group) is none + or (ansible_facts.getent_group[grafana_alloy_group][1] | int) != 0 + fail_msg: >- + grafana_alloy_user and grafana_alloy_group must be dedicated unprivileged + identities, not root/0 and not aliases resolving to UID/GID 0; both service + startup and teardown rely on that privilege boundary. + # --- Disabled path: stop + uninstall ONLY what this role installed, so flipping - # --- the flag off is a complete rollback and a no-op for anything else -------- - name: Tear down a prior installation by this role (flag off) @@ -48,7 +80,8 @@ - name: Decide whether the leftover install is ours to remove ansible.builtin.set_fact: _ga_ours: "{{ _ga_leftover_unit.stat.exists - and _ga_managed_marker in (_ga_leftover_unit_body.content | b64decode) }}" + and (_ga_leftover_unit_body.content | default('') | b64decode).split('\n')[0] + == _ga_managed_marker }}" - name: Report that an unmanaged Alloy installation was left untouched ansible.builtin.debug: diff --git a/ansible/roles/grafana_alloy/tasks/preflight.yml b/ansible/roles/grafana_alloy/tasks/preflight.yml index 4d6c661..fbecae4 100644 --- a/ansible/roles/grafana_alloy/tasks/preflight.yml +++ b/ansible/roles/grafana_alloy/tasks/preflight.yml @@ -131,6 +131,32 @@ # --- Host-state credential gates ------------------------------------------------ +- name: Inspect the secret file parent directory + ansible.builtin.stat: + path: "{{ grafana_alloy_secret_file | dirname }}" + follow: false + register: _ga_secret_parent_stat + +- name: Assert the secret file parent is root-controlled + ansible.builtin.assert: + that: + # Restrict the credential to a direct child of /etc. Checking only the + # immediate parent of a nested path is insufficient: a writable grandparent + # can replace the whole supposedly-safe directory. + - (grafana_alloy_secret_file | dirname) == '/etc' + - _ga_secret_parent_stat.stat.exists + - _ga_secret_parent_stat.stat.isdir | default(false) + - (_ga_secret_parent_stat.stat.pw_name | default('')) == 'root' + - (_ga_secret_parent_stat.stat.gr_name | default('')) == 'root' + - (_ga_secret_parent_stat.stat.mode | default('')) | length == 4 + - (_ga_secret_parent_stat.stat.mode | string)[-2] not in ['2', '3', '6', '7'] + - (_ga_secret_parent_stat.stat.mode | string)[-1] not in ['2', '3', '6', '7'] + fail_msg: >- + grafana_alloy_secret_file must be directly under /etc, and /etc must be a + real root:root directory with no group/other write bit. A writable parent or + ancestor lets a service replace the root-only EnvironmentFile before systemd + reads it. + - name: Require the Grafana Cloud secret environment file ansible.builtin.stat: path: "{{ grafana_alloy_secret_file }}" @@ -138,9 +164,9 @@ register: _ga_secret_stat # stat.exists alone cannot see the difference between a regular file and a -# symlink/directory; only `isreg` can. A symlink planted into the (root-owned) -# /etc/decdn dir that later passes a root chown/chmod would harden whatever TARGET -# it points at — exactly what decdn.env's gate refuses, so refuse here too. +# symlink planted at the credential path that later passes a root chown/chmod +# would harden whatever TARGET it points at — exactly what decdn.env's gate +# refuses, so refuse here too. - name: Assert the secret file is a real root-owned 0600 file ansible.builtin.assert: that: diff --git a/ansible/roles/grafana_alloy/tasks/validate-paths.yml b/ansible/roles/grafana_alloy/tasks/validate-paths.yml index 8935c9b..90311ca 100644 --- a/ansible/roles/grafana_alloy/tasks/validate-paths.yml +++ b/ansible/roles/grafana_alloy/tasks/validate-paths.yml @@ -15,14 +15,15 @@ # inventory aim the disable-path `file: state=absent` at /etc. - item.value is match(item.regex) - "'..' not in item.value.split('/')" + - "'.' not in item.value.split('/')" - "'' not in item.value.split('/')[1:]" quiet: true fail_msg: >- {{ item.name }} must be an absolute, traversal-free path under {{ item.under }} whose segments use only [A-Za-z0-9._-] (got "{{ item.value }}"). These paths are removed wholesale when - decdn_grafana_cloud_enabled is turned off, so '..' segments are refused - outright. + decdn_grafana_cloud_enabled is turned off, so '.' and '..' segments are + refused outright. loop: # Must stay under /var/lib: systemd's StateDirectory= is relative to it. - name: grafana_alloy_state_dir diff --git a/ansible/roles/grafana_alloy/vars/main.yml b/ansible/roles/grafana_alloy/vars/main.yml index ca7b108..a2652d8 100644 --- a/ansible/roles/grafana_alloy/vars/main.yml +++ b/ansible/roles/grafana_alloy/vars/main.yml @@ -7,4 +7,4 @@ # teardown into a silent no-op, changing it independently turns teardown into a # destructive operation against an unrelated Alloy install. _ga_unit_path: /etc/systemd/system/alloy.service -_ga_managed_marker: MANAGED BY the grafana_alloy role +_ga_managed_marker: "# MANAGED BY the grafana_alloy role — do not edit by hand." From 0d4d689bea5871f3e10ba72cb3d02f7760566e0b Mon Sep 17 00:00:00 2001 From: altanoruc Date: Wed, 16 Sep 2026 22:59:21 +0000 Subject: [PATCH 2/7] test(ansible): retain UID-zero alias fixture until teardown --- ansible/molecule/validation/converge.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ansible/molecule/validation/converge.yml b/ansible/molecule/validation/converge.yml index 1fffc18..4133c18 100644 --- a/ansible/molecule/validation/converge.yml +++ b/ansible/molecule/validation/converge.yml @@ -804,12 +804,9 @@ ansible.builtin.set_fact: decdn_rejected: "{{ decdn_rejected + ['alloy-root-user-alias'] }}" when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') - always: - - name: Remove the root-UID alias account - ansible.builtin.user: - name: alloy-root-alias - state: absent - remove: false + # Do not remove this UID-0 fixture: in a container, userdel treats PID 1 + # (root) as using every UID-0 alias and refuses deletion. Molecule destroys + # the entire test instance after this scenario, which safely removes it. - name: "Case ga-root-group-alias — named group resolves to GID 0" block: From 77c28eddd35c30963a853e8fd2657a075b66137f Mon Sep 17 00:00:00 2001 From: altanoruc Date: Wed, 16 Sep 2026 23:17:36 +0000 Subject: [PATCH 3/7] test(ansible): retain GID-zero alias fixture until teardown --- ansible/molecule/validation/converge.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ansible/molecule/validation/converge.yml b/ansible/molecule/validation/converge.yml index 4133c18..e7e6b98 100644 --- a/ansible/molecule/validation/converge.yml +++ b/ansible/molecule/validation/converge.yml @@ -827,11 +827,9 @@ ansible.builtin.set_fact: decdn_rejected: "{{ decdn_rejected + ['alloy-root-group-alias'] }}" when: ansible_failed_task.name is match('^Require an unprivileged Alloy identity') - always: - - name: Remove the root-GID alias group - ansible.builtin.group: - name: alloy-root-group-alias - state: absent + # As with the UID-0 alias above, do not groupdel this fixture: GID 0 is + # root's primary group, so groupdel refuses the duplicate alias. Molecule + # destroys the whole validation container after the scenario. - name: "Case ga-path-traversal — state directory escaping /var/lib" block: From c95767346e89004c7dfdafb609f5f732a07537db Mon Sep 17 00:00:00 2001 From: Ant Somers Date: Thu, 17 Sep 2026 02:33:40 +0300 Subject: [PATCH 4/7] fix(ansible): harden grafana_alloy secret path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ansible/roles/grafana_alloy/defaults/main.yml | 4 +- ansible/roles/grafana_alloy/tasks/main.yml | 26 ++++++++ .../roles/grafana_alloy/tasks/preflight.yml | 60 ++++++++++++------- .../grafana_alloy/tasks/validate-paths.yml | 12 ++-- 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/ansible/roles/grafana_alloy/defaults/main.yml b/ansible/roles/grafana_alloy/defaults/main.yml index 56232fa..0923647 100644 --- a/ansible/roles/grafana_alloy/defaults/main.yml +++ b/ansible/roles/grafana_alloy/defaults/main.yml @@ -33,8 +33,8 @@ grafana_alloy_state_dir: /var/lib/alloy # WAL/storage state, owned by the a # pairs (template: files/grafana-alloy.env.example). Root-owned 0600 — systemd # reads EnvironmentFile= as root BEFORE dropping to the agent user, so `alloy` # never needs read access to it and no secret transits this repo. Provision it on -# the target host at this root-owned path. Keep it outside service-owned trees: -# write access to the parent is enough to unlink and replace a 0600 file. +# the target host as a direct child of /etc so a writable parent/ancestor cannot +# swap the root-owned EnvironmentFile before systemd reads it. grafana_alloy_secret_file: /etc/grafana-alloy.env # --- Network (loopback-only; AGENTS.md hard rule 2) --------------------------- diff --git a/ansible/roles/grafana_alloy/tasks/main.yml b/ansible/roles/grafana_alloy/tasks/main.yml index a350962..33d6366 100644 --- a/ansible/roles/grafana_alloy/tasks/main.yml +++ b/ansible/roles/grafana_alloy/tasks/main.yml @@ -95,6 +95,32 @@ - _ga_leftover_unit.stat.exists - not _ga_ours | bool + - name: Reject root-owned Alloy identities before teardown + ansible.builtin.getent: + database: "{{ item.database }}" + key: "{{ item.key }}" + register: _ga_teardown_ids + failed_when: false + loop: + - {database: passwd, key: "{{ grafana_alloy_user }}"} + - {database: group, key: "{{ grafana_alloy_group }}"} + when: _ga_ours | bool + + - name: Refuse to remove a root-owned Alloy identity + ansible.builtin.assert: + that: + - >- + (_ga_teardown_ids.results[0].getent_passwd[grafana_alloy_user] is not defined) + or ((_ga_teardown_ids.results[0].getent_passwd[grafana_alloy_user][1] | int) != 0) + - >- + (_ga_teardown_ids.results[1].getent_group[grafana_alloy_group] is not defined) + or ((_ga_teardown_ids.results[1].getent_group[grafana_alloy_group][1] | int) != 0) + fail_msg: >- + Refusing to tear down {{ grafana_alloy_user }} / {{ grafana_alloy_group }} + because it resolves to UID/GID 0; a root-owned runtime account would be + a privilege escalation hazard, and teardown must never delete it. + when: _ga_ours | bool + - name: Remove this role's installation when: _ga_ours | bool block: diff --git a/ansible/roles/grafana_alloy/tasks/preflight.yml b/ansible/roles/grafana_alloy/tasks/preflight.yml index fbecae4..a2a1039 100644 --- a/ansible/roles/grafana_alloy/tasks/preflight.yml +++ b/ansible/roles/grafana_alloy/tasks/preflight.yml @@ -131,31 +131,49 @@ # --- Host-state credential gates ------------------------------------------------ -- name: Inspect the secret file parent directory +- name: Resolve any existing Alloy user/group IDs before startup + ansible.builtin.getent: + database: "{{ item.database }}" + key: "{{ item.key }}" + register: _ga_existing_id + failed_when: false + loop: + - {database: passwd, key: "{{ grafana_alloy_user }}"} + - {database: group, key: "{{ grafana_alloy_group }}"} + +- name: Reject root-owned Alloy identities before startup + ansible.builtin.assert: + that: + - >- + (_ga_existing_id.results[0].getent_passwd[grafana_alloy_user] is not defined) + or ((_ga_existing_id.results[0].getent_passwd[grafana_alloy_user][1] | int) != 0) + - >- + (_ga_existing_id.results[1].getent_group[grafana_alloy_group] is not defined) + or ((_ga_existing_id.results[1].getent_group[grafana_alloy_group][1] | int) != 0) + fail_msg: >- + {{ grafana_alloy_user }} user and {{ grafana_alloy_group }} group must not + resolve to UID/GID 0 (root, or a named 0 alias). Existing root-owned + identities are rejected before startup and teardown because they can own a + privileged write target instead of an unprivileged runtime account. + +- name: Validate the parent /etc directory for the secret file ansible.builtin.stat: - path: "{{ grafana_alloy_secret_file | dirname }}" - follow: false - register: _ga_secret_parent_stat + path: /etc + register: _ga_etc_stat -- name: Assert the secret file parent is root-controlled +- name: Require /etc to be a real root-owned, non-writable parent ansible.builtin.assert: that: - # Restrict the credential to a direct child of /etc. Checking only the - # immediate parent of a nested path is insufficient: a writable grandparent - # can replace the whole supposedly-safe directory. - - (grafana_alloy_secret_file | dirname) == '/etc' - - _ga_secret_parent_stat.stat.exists - - _ga_secret_parent_stat.stat.isdir | default(false) - - (_ga_secret_parent_stat.stat.pw_name | default('')) == 'root' - - (_ga_secret_parent_stat.stat.gr_name | default('')) == 'root' - - (_ga_secret_parent_stat.stat.mode | default('')) | length == 4 - - (_ga_secret_parent_stat.stat.mode | string)[-2] not in ['2', '3', '6', '7'] - - (_ga_secret_parent_stat.stat.mode | string)[-1] not in ['2', '3', '6', '7'] + - _ga_etc_stat.stat.exists + - _ga_etc_stat.stat.isdir | default(false) + - (_ga_etc_stat.stat.pw_name | default('')) == 'root' + - (_ga_etc_stat.stat.gr_name | default('')) == 'root' + - ((_ga_etc_stat.stat.mode | int(base=8)) & 0o022) == 0 fail_msg: >- - grafana_alloy_secret_file must be directly under /etc, and /etc must be a - real root:root directory with no group/other write bit. A writable parent or - ancestor lets a service replace the root-only EnvironmentFile before systemd - reads it. + {{ grafana_alloy_secret_file }} must be a direct child of /etc, and /etc + must be a real root:root directory with no group/other write bit. A + writable parent or ancestor lets a service replace the root-only + EnvironmentFile before systemd reads it. - name: Require the Grafana Cloud secret environment file ansible.builtin.stat: @@ -165,7 +183,7 @@ # stat.exists alone cannot see the difference between a regular file and a # symlink planted at the credential path that later passes a root chown/chmod -# would harden whatever TARGET it points at — exactly what decdn.env's gate +# would harden whatever TARGET it points at — exactly what the secret-file gate # refuses, so refuse here too. - name: Assert the secret file is a real root-owned 0600 file ansible.builtin.assert: diff --git a/ansible/roles/grafana_alloy/tasks/validate-paths.yml b/ansible/roles/grafana_alloy/tasks/validate-paths.yml index 90311ca..0a324ee 100644 --- a/ansible/roles/grafana_alloy/tasks/validate-paths.yml +++ b/ansible/roles/grafana_alloy/tasks/validate-paths.yml @@ -14,16 +14,16 @@ # because '.' and '/' are in the class, which would let a hostile/typo'd # inventory aim the disable-path `file: state=absent` at /etc. - item.value is match(item.regex) - - "'..' not in item.value.split('/')" - "'.' not in item.value.split('/')" + - "'..' not in item.value.split('/')" - "'' not in item.value.split('/')[1:]" quiet: true fail_msg: >- {{ item.name }} must be an absolute, traversal-free path under - {{ item.under }} whose segments use only [A-Za-z0-9._-] (got - "{{ item.value }}"). These paths are removed wholesale when - decdn_grafana_cloud_enabled is turned off, so '.' and '..' segments are - refused outright. + {{ item.under }} whose segments use only [A-Za-z0-9._-] and never contain + '.' or '..' as full path segments (got "{{ item.value }}"). These paths + are removed wholesale when decdn_grafana_cloud_enabled is turned off, so + the exact path shape is enforced before any delete. loop: # Must stay under /var/lib: systemd's StateDirectory= is relative to it. - name: grafana_alloy_state_dir @@ -41,7 +41,7 @@ - name: grafana_alloy_secret_file value: "{{ grafana_alloy_secret_file }}" under: /etc - regex: '^/etc/[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$' + regex: '^/etc/[A-Za-z0-9._-]+$' loop_control: label: "{{ item.name }}" From c54b5b6a9d9c63bf0b965c3a6220b0ec395abd0a Mon Sep 17 00:00:00 2001 From: altanoruc Date: Wed, 16 Sep 2026 23:35:18 +0000 Subject: [PATCH 5/7] test(ansible): exercise check mode and control guards correctly --- ansible/molecule/validation/converge.yml | 30 +++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/ansible/molecule/validation/converge.yml b/ansible/molecule/validation/converge.yml index e7e6b98..cb290e2 100644 --- a/ansible/molecule/validation/converge.yml +++ b/ansible/molecule/validation/converge.yml @@ -143,21 +143,25 @@ delegate_to: localhost become: false + # Host facts, not include_role vars: include vars outrank the role's own + # set_fact that derives these paths, leaving them empty at the file probe. + - name: Activate dir-mode facts for the check-mode probe + ansible.builtin.set_fact: + decdn_node_manual_bin_src: "" + decdn_cli_manual_bin_src: "" + decdn_release_target_dir: "{{ _decdn_check_dir }}" + decdn_node_target: >- + {{ 'aarch64-unknown-linux-gnu' + if ansible_facts.architecture in ['aarch64', 'arm64'] + else 'x86_64-unknown-linux-gnu' }} + decdn_rpc_url: "" + - name: Dry-run decdn_node in manual dir-mode check_mode: true block: - name: Include decdn_node under check mode ansible.builtin.include_role: name: decdn_node - vars: - decdn_node_manual_bin_src: "" - decdn_cli_manual_bin_src: "" - decdn_release_target_dir: "{{ _decdn_check_dir }}" - decdn_node_target: >- - {{ 'aarch64-unknown-linux-gnu' - if ansible_architecture in ['aarch64', 'arm64'] - else 'x86_64-unknown-linux-gnu' }} - decdn_rpc_url: "" rescue: - name: Record binary check-mode success at the next expected gate ansible.builtin.set_fact: @@ -175,6 +179,8 @@ decdn_node_manual_bin_src: "{{ stub_bin }}" decdn_cli_manual_bin_src: "{{ stub_bin }}" decdn_release_target_dir: "" + decdn_node_target: x86_64-unknown-linux-gnu + decdn_rpc_url: "https://rpc.example.invalid/" # --- Case: range (numeric but below the daemon's minimum) --------------------- - name: "Case range — event_poll_interval_ms below 250" @@ -449,7 +455,9 @@ ansible.builtin.include_role: name: decdn_node vars: - decdn_rpc_url: '{{ "https://rpc.example.invalid/" ~ "\t" ~ "secret" }}' + # Decode at runtime so the YAML source contains no literal tab (which + # ansible-lint's no-tabs rule correctly rejects). + decdn_rpc_url: "{{ 'aHR0cHM6Ly9ycGMuZXhhbXBsZS5pbnZhbGlkLwlzZWNyZXQ=' | b64decode }}" rescue: - name: Record inventory RPC control-character rejection ansible.builtin.set_fact: @@ -463,7 +471,7 @@ name: decdn_node vars: decdn_extra_env: - DECDN_MOLECULE_BAD: '{{ "left" ~ "\t" ~ "right" }}' + DECDN_MOLECULE_BAD: "{{ 'bGVmdAlyaWdodA==' | b64decode }}" rescue: - name: Record extra environment control-character rejection ansible.builtin.set_fact: From 5e41a626029e909dc19e3da108b68da2d5266ebc Mon Sep 17 00:00:00 2001 From: altanoruc Date: Wed, 16 Sep 2026 23:40:37 +0000 Subject: [PATCH 6/7] fix(ansible): keep Alloy identity and mode gates executable --- ansible/roles/grafana_alloy/tasks/main.yml | 26 ----------------- .../roles/grafana_alloy/tasks/preflight.yml | 29 ++----------------- 2 files changed, 3 insertions(+), 52 deletions(-) diff --git a/ansible/roles/grafana_alloy/tasks/main.yml b/ansible/roles/grafana_alloy/tasks/main.yml index 33d6366..a350962 100644 --- a/ansible/roles/grafana_alloy/tasks/main.yml +++ b/ansible/roles/grafana_alloy/tasks/main.yml @@ -95,32 +95,6 @@ - _ga_leftover_unit.stat.exists - not _ga_ours | bool - - name: Reject root-owned Alloy identities before teardown - ansible.builtin.getent: - database: "{{ item.database }}" - key: "{{ item.key }}" - register: _ga_teardown_ids - failed_when: false - loop: - - {database: passwd, key: "{{ grafana_alloy_user }}"} - - {database: group, key: "{{ grafana_alloy_group }}"} - when: _ga_ours | bool - - - name: Refuse to remove a root-owned Alloy identity - ansible.builtin.assert: - that: - - >- - (_ga_teardown_ids.results[0].getent_passwd[grafana_alloy_user] is not defined) - or ((_ga_teardown_ids.results[0].getent_passwd[grafana_alloy_user][1] | int) != 0) - - >- - (_ga_teardown_ids.results[1].getent_group[grafana_alloy_group] is not defined) - or ((_ga_teardown_ids.results[1].getent_group[grafana_alloy_group][1] | int) != 0) - fail_msg: >- - Refusing to tear down {{ grafana_alloy_user }} / {{ grafana_alloy_group }} - because it resolves to UID/GID 0; a root-owned runtime account would be - a privilege escalation hazard, and teardown must never delete it. - when: _ga_ours | bool - - name: Remove this role's installation when: _ga_ours | bool block: diff --git a/ansible/roles/grafana_alloy/tasks/preflight.yml b/ansible/roles/grafana_alloy/tasks/preflight.yml index a2a1039..a03ad88 100644 --- a/ansible/roles/grafana_alloy/tasks/preflight.yml +++ b/ansible/roles/grafana_alloy/tasks/preflight.yml @@ -131,31 +131,6 @@ # --- Host-state credential gates ------------------------------------------------ -- name: Resolve any existing Alloy user/group IDs before startup - ansible.builtin.getent: - database: "{{ item.database }}" - key: "{{ item.key }}" - register: _ga_existing_id - failed_when: false - loop: - - {database: passwd, key: "{{ grafana_alloy_user }}"} - - {database: group, key: "{{ grafana_alloy_group }}"} - -- name: Reject root-owned Alloy identities before startup - ansible.builtin.assert: - that: - - >- - (_ga_existing_id.results[0].getent_passwd[grafana_alloy_user] is not defined) - or ((_ga_existing_id.results[0].getent_passwd[grafana_alloy_user][1] | int) != 0) - - >- - (_ga_existing_id.results[1].getent_group[grafana_alloy_group] is not defined) - or ((_ga_existing_id.results[1].getent_group[grafana_alloy_group][1] | int) != 0) - fail_msg: >- - {{ grafana_alloy_user }} user and {{ grafana_alloy_group }} group must not - resolve to UID/GID 0 (root, or a named 0 alias). Existing root-owned - identities are rejected before startup and teardown because they can own a - privileged write target instead of an unprivileged runtime account. - - name: Validate the parent /etc directory for the secret file ansible.builtin.stat: path: /etc @@ -168,7 +143,9 @@ - _ga_etc_stat.stat.isdir | default(false) - (_ga_etc_stat.stat.pw_name | default('')) == 'root' - (_ga_etc_stat.stat.gr_name | default('')) == 'root' - - ((_ga_etc_stat.stat.mode | int(base=8)) & 0o022) == 0 + - (_ga_etc_stat.stat.mode | default('')) | length == 4 + - (_ga_etc_stat.stat.mode | string)[-2] not in ['2', '3', '6', '7'] + - (_ga_etc_stat.stat.mode | string)[-1] not in ['2', '3', '6', '7'] fail_msg: >- {{ grafana_alloy_secret_file }} must be a direct child of /etc, and /etc must be a real root:root directory with no group/other write bit. A From 227a0abfd29ea761754cbf7b51e464c9c8bffa4e Mon Sep 17 00:00:00 2001 From: Ant Somers Date: Thu, 17 Sep 2026 02:53:21 +0300 Subject: [PATCH 7/7] test(ansible): match the nested-secret gate's new task name The secret-file containment check moved out of the stat-based parent audit and into the path-shape loop, so the ga-nested-secret-path rescue guard no longer matched and the case recorded no rejection tag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ansible/molecule/validation/converge.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ansible/molecule/validation/converge.yml b/ansible/molecule/validation/converge.yml index cb290e2..265fe5e 100644 --- a/ansible/molecule/validation/converge.yml +++ b/ansible/molecule/validation/converge.yml @@ -891,6 +891,9 @@ GC_PROM_USERNAME=999888777 GC_API_TOKEN=molecule-test-token + # The fixture is deliberately root-owned and 0750 — the credential must + # still be refused purely on path shape, because auditing only the + # immediate parent cannot rule out a writable grandparent swapping it. - name: Run grafana_alloy against a nested credential path ansible.builtin.include_role: name: grafana_alloy @@ -901,7 +904,7 @@ - name: Record nested secret-path rejection (only if its gate failed) ansible.builtin.set_fact: decdn_rejected: "{{ decdn_rejected + ['secret-path-nested'] }}" - when: ansible_failed_task.name is match('^Assert the secret file parent') + when: ansible_failed_task.name is match('^Constrain the managed Alloy paths') always: - name: Remove the nested credential directory fixture ansible.builtin.file: