Repository navigation
docs(ci-syslog-watcher): document config writer trust boundary - #9882
Open
Calvin S. (calvin197) wants to merge 3 commits into
Open
Calvin S. (calvin197) wants to merge 3 commits into
Calvin S. (calvin197) wants to merge 3 commits into
Conversation
…slog config copy
ci-syslog-watcher.sh copies /var/run/mdsd-ci/70-rsyslog-forward-mdsd-ci.conf
into /etc/rsyslog.d and restarts rsyslog without validating its content.
This is intentional, not an oversight:
- The trust boundary for this script is the /var/run/mdsd-ci directory
itself, which is populated by the privileged Container Insights
(mdsd/AMA) add-on via a hostPath mount - a component deployed and
access-controlled entirely outside this repo. AgentBaker has no
visibility into, or control over, who can write to that directory.
- Anything capable of writing into /var/run/mdsd-ci already has
node-level privilege equivalent to what content validation here
could additionally restrict, so adding it would not meaningfully
reduce risk - it would only create a false sense of security.
- The real control point is the add-on's own hostPath/RBAC
configuration, which is out of scope for this repo.
No functional change - in-place comment only. Verified: bash -n and
shellcheck both pass with no warnings. No existing ShellSpec coverage
for this script to regress.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981
Contributor
Windows Unit Test Results 3 files 18 suites 1m 12s ⏱️ Results for commit 47e7ea8. ♻️ This comment has been updated with latest results. |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The risk acceptance depends on external writer privileges and access controls that require confirmation from the add-on owner.
1 open issue
What changed in this PR
Documents the risk-acceptance rationale for AgentBaker’s Container Insights syslog watcher copying configuration without content validation.
Changes:
- Adds an inline explanation of the intended trust boundary and external add-on access controls.
- Leaves configuration copying and rsyslog restart behavior unchanged.
| File | Description |
|---|---|
parts/linux/cloud-init/artifacts/ci-syslog-watcher.sh |
Adds security rationale comments around the existing configuration copy. |
Review effort: Balanced
Copilot's review on PR #9882 correctly pointed out that the risk-acceptance comment overclaimed: write access to the /var/run/mdsd-ci hostPath does not inherently guarantee node-level privilege for every possible writer - a workload could in principle be scoped to write only that one path. Reworded to state this as a deployment assumption that is confirmed by the add-on owner, rather than an inherent property of directory access. No functional change - comment wording only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Risk acceptance for ADO 37518910 depends on external deployment controls and confirmation that all hostPath writers already have equivalent node privileges, requiring security or add-on owner validation unavailable in this repository.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
|
|
||
| if [ "$status" = "add" ]; then | ||
| echo "Status changed to $status." | ||
| # By design: the rsyslog config dropped here is not content-validated before being |
Contributor
There was a problem hiding this comment.
is this actually necessary?
Cameron Meissner (cameronmeissner)
approved these changes
Oct 29, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bb31c5eb-70b8-418b-93cc-3c2bc38e0951
Calvin S. (calvin197)
changed the title
Contributor
|
Contributor
|
Contributor
|
Cameron Meissner (cameronmeissner)
approved these changes
Oct 29, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
A security review flagged
ci-syslog-watcher.shcopying the Container Insights rsyslog configuration into/etc/rsyslog.dand restarting rsyslog without content validation (ADO finding 37518910).This comment-only PR keeps the trust requirement next to that handoff so future maintainers and AI-assisted reviewers can see the external deployment dependency without reconstructing the context:
/var/run/mdsd-cito configure host rsyslog. Deployment owners must restrict write access to components trusted for that role.This is documentation, not a mitigation or a completed risk-acceptance decision. It does not establish that content validation has no possible defense-in-depth value, or guarantee that future scans will stop reporting this path.
No functional or behavioral change. Validation for this revision:
bash -n,shellcheck, andgit diff --checkpassed. Comparing the script before and after with full-line comments removed confirmed that all non-comment lines are unchanged. The watcher itself was not executed.Which issue(s) this PR fixes:
Documents context for the tracked finding; it does not remediate or close it.
ADO work items (msazure/CloudNativeCompute): 37518910 (parent: 37518878)