feat: Write roles fingerprints to /var/log/sysroles.jsonl [citest_skip] - #230
Conversation
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSummaryThe ChangesStructured fingerprint logging
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
library/sr_fingerprint.py (2)
270-274: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueUse backslash escaping for embedded quotes.
The current code doubles an embedded
"character. Commonkey=valueparsers, including logfmt-style parsers used with rsyslog pipelines, expect\". A value such asa"bbecomesrole_path="a""b", which those parsers truncate.♻️ Proposed change
def _format_fingerprint_key_value(field, value): text = "" if value is None else str(value) if any(char in text for char in ' "='): - return '%s="%s"' % (field, text.replace('"', '""')) + return '%s="%s"' % (field, text.replace("\\", "\\\\").replace('"', '\\"')) return "%s=%s" % (field, text)The test at
tests/unit/test_sr_fingerprint.py:120-124still passes, because that value contains no quote character.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@library/sr_fingerprint.py` around lines 270 - 274, Update _format_fingerprint_key_value to escape embedded double quotes with a backslash when constructing quoted values, producing parser-compatible output such as role_path="a\"b". Preserve the existing quoting conditions and handling for None and unquoted values.
211-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider excluding the
.lockfile from the log directory listing contract.The lock file is created next to the log file as
sysroles.jsonl.lock. Downstream consumers that glob/var/log/sysroles*will pick it up. The lock file is also never removed and is created with default permissions.This is acceptable behavior. Document the
.locksidecar in the module description so downstream users do not treat it as data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@library/sr_fingerprint.py` around lines 211 - 224, Update the module description to document that _write_jsonl_log creates a persistent .lock sidecar next to the JSONL log, such as sysroles.jsonl.lock, and that directory listings may include it. Clarify that downstream consumers should exclude this lock file from data processing; do not change the locking implementation.tests/unit/test_sr_fingerprint.py (1)
126-143: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the
.locksidecar file in the cleanup blocks.
_write_jsonl_logcreates<log_file>.lock. Thefinallyblocks in this test and intest_write_jsonl_log_preserves_types,test_trim_removes_oldest_lines,test_trim_disabled_when_zero, andtest_trim_no_op_when_under_limitunlink only the log file. Each run leaves a stale lock file in the temporary directory.Extract a helper that removes both paths.
🧹 Proposed helper
def _cleanup_log(log_file): for path in (log_file, log_file + ".lock"): try: os.unlink(path) except OSError: passThen call
_cleanup_log(log_file)in eachfinallyblock instead ofos.unlink(log_file).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_sr_fingerprint.py` around lines 126 - 143, Extract a _cleanup_log(log_file) helper in the test module that attempts to remove both the log file and its “.lock” sidecar while ignoring missing-file errors. Replace direct os.unlink(log_file) cleanup in the finally blocks of test_write_jsonl_log_appends_valid_json_lines, test_write_jsonl_log_preserves_types, test_trim_removes_oldest_lines, test_trim_disabled_when_zero, and test_trim_no_op_when_under_limit with this helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.coderabbit.yaml:
- Around line 42-43: The commit-title guidance and validation currently disagree
about scoped Conventional Commit titles. Choose one policy and apply it
consistently: either update `.coderabbit.yaml` guidance and examples to include
the optional `(scope)` form, or add the `scope-empty` rule in `.commitlintrc.js`
to reject scopes; modify both named sites as needed to reflect the chosen
behavior.
In @.github/workflows/changelog_to_tag.yml:
- Line 25: Disable checkout credential persistence by adding
persist-credentials: false to the checkout steps in
.github/workflows/changelog_to_tag.yml (lines 25-25),
.github/workflows/codeql.yml (lines 36-37), .github/workflows/codespell.yml
(lines 15-16), .github/workflows/markdownlint.yml (lines 30-31),
.github/workflows/python-unit-test.yml (lines 45-46), and
.github/workflows/test_converting_readme.yml (lines 31-32). In
changelog_to_tag.yml, configure checkout with an explicit token for the later
git fetch --all --tags operation before disabling persistence; leave the other
sites with only the persistence change.
In @.github/workflows/pr-title-lint.yml:
- Around line 25-26: Update the “Install conventional-commit linter” workflow
step to use locked dependencies: add `@commitlint/config-conventional` and
`@commitlint/cli` to a checked-in package.json with its generated lockfile, then
replace npm install with npm ci so PR runs use the locked versions.
- Around line 21-26: Update the actions/checkout step in the PR-title lint
workflow to set persist-credentials to false, while retaining the existing full
fetch configuration and subsequent npm install step.
In @.github/workflows/qemu-kvm-integration-tests.yml:
- Around line 53-54: Update the actions/checkout step in the workflow to set
persist-credentials to false, ensuring checkout credentials are not retained for
subsequent test dependency and execution steps.
- Around line 115-135: Update the Podman compatibility step around the “Update
podman to 5.x” workflow action so it no longer rewrites Noble sources to the EOL
Plucky suite. Use a currently supported Ubuntu suite that provides the required
Podman 5 packages, or change the Plucky source to old-releases.ubuntu.com as a
temporary fallback while preserving the existing package pinning and
installation behavior.
In @.github/workflows/tft.yml:
- Around line 38-39: Update each actions/checkout@v6 step to set
persist-credentials to read-only: .github/workflows/tft.yml lines 38-39 and
51-52, and .github/workflows/woke.yml lines 13-14. Preserve the existing
checkout behavior and permissions.
In `@library/sr_fingerprint.py`:
- Around line 286-317: Format the affected code with Black and satisfy Ruff
B007: in library/sr_fingerprint.py lines 286-317, wrap the long fail_json call
and keep the log-file failure msg expression on one formatted line; in
tests/unit/test_sr_fingerprint.py lines 295-295 and 313-313, wrap the long
assertIn calls, and in lines 206-206 and 222-222 rename the unused loop variable
i to _i. Run tox -e black,flake8 before committing.
- Around line 196-202: Update the trimming logic around tempfile.mkstemp and
os.rename to preserve the existing log file’s mode and ownership: capture the
original file metadata, apply its permission bits using stat, and copy its
user/group ownership to the temporary file before replacement. Add the required
stat import and keep the atomic rename behavior unchanged.
- Line 10: Update the short_description metadata in sr_fingerprint.py to remove
its trailing period, leaving the rest of the description unchanged.
In `@tests/unit/test_sr_fingerprint.py`:
- Around line 278-295: Update
test_handle_fingerprint_write_failure_calls_fail_json to force _write_jsonl_log
to raise an IOError instead of relying on an unwritable filesystem path.
Temporarily replace sr_fingerprint._write_jsonl_log, restore the original in a
finally block, and retain the assertion that _handle_fingerprint invokes
fail_json with the expected message.
- Line 16: Update the import setup for tests/unit/test_sr_fingerprint.py so
sr_fingerprint resolves reliably during test collection: either add library to
the tox test configuration path or replace the import with the appropriate
package-relative form. Preserve the existing test behavior and ensure collection
does not depend on an implicit sys.path entry.
---
Nitpick comments:
In `@library/sr_fingerprint.py`:
- Around line 270-274: Update _format_fingerprint_key_value to escape embedded
double quotes with a backslash when constructing quoted values, producing
parser-compatible output such as role_path="a\"b". Preserve the existing quoting
conditions and handling for None and unquoted values.
- Around line 211-224: Update the module description to document that
_write_jsonl_log creates a persistent .lock sidecar next to the JSONL log, such
as sysroles.jsonl.lock, and that directory listings may include it. Clarify that
downstream consumers should exclude this lock file from data processing; do not
change the locking implementation.
In `@tests/unit/test_sr_fingerprint.py`:
- Around line 126-143: Extract a _cleanup_log(log_file) helper in the test
module that attempts to remove both the log file and its “.lock” sidecar while
ignoring missing-file errors. Replace direct os.unlink(log_file) cleanup in the
finally blocks of test_write_jsonl_log_appends_valid_json_lines,
test_write_jsonl_log_preserves_types, test_trim_removes_oldest_lines,
test_trim_disabled_when_zero, and test_trim_no_op_when_under_limit with this
helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 07c75c8e-b2bb-44ab-9137-75ad097a8ba3
📒 Files selected for processing (19)
.coderabbit.yaml.commitlintrc.js.github/workflows/ansible-lint.yml.github/workflows/ansible-managed-var-comment.yml.github/workflows/ansible-test.yml.github/workflows/build_docs.yml.github/workflows/changelog_to_tag.yml.github/workflows/codeql.yml.github/workflows/codespell.yml.github/workflows/markdownlint.yml.github/workflows/pr-title-lint.yml.github/workflows/python-unit-test.yml.github/workflows/qemu-kvm-integration-tests.yml.github/workflows/test_converting_readme.yml.github/workflows/tft.yml.github/workflows/weekly_ci.yml.github/workflows/woke.ymllibrary/sr_fingerprint.pytests/unit/test_sr_fingerprint.py
| - Format: <type>: <description> or <type>!: <description> for breaking changes | ||
| - Valid types: Refer to the 'type-enum' rule in .commitlintrc.js file for the complete list of allowed types |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | grep -E '(^|/)(\.coderabbit\.yaml|\.commitlintrc\.js|commitlint)' || true
echo
echo ".coderabbit.yaml relevant:"
if [ -f .coderabbit.yaml ]; then
nl -ba .coderabbit.yaml | sed -n '30,55p'
fi
echo
echo ".commitlintrc.js relevant:"
if [ -f .commitlintrc.js ]; then
nl -ba .commitlintrc.js | sed -n '1,80p'
fi
echo
echo "Commitlint usages/workflows:"
rg -n "commitlint|commit-lint|CODE_RABBIT|coderabbit|push|pull_request|pr-title|PR_TITLE|scope-empty|type-enum" .github . 2>/dev/null | head -200 || trueRepository: linux-system-roles/bootloader
Length of output: 277
🌐 Web query:
commitlint rules scope-empty array always default behavior missing scopes
💡 Result:
In commitlint, the scope-empty rule governs whether a scope is required in the commit message header [1]. The default behavior depends on your configuration, as commitlint does not enforce this rule unless explicitly enabled in your configuration file [1]. When configuring the scope-empty rule, it is defined using an array with the format [level, value, [options]] [1][2]: 1. [2, 'never']: This configuration makes the scope mandatory; a commit message without a scope will trigger an error [1][2]. 2. [2, 'always']: This configuration forbids the use of a scope; any commit message containing a scope will trigger an error [1]. If the rule is set to or omitted entirely, the rule is disabled and commitlint will not enforce any requirements regarding the presence or absence of a scope [1]. Regarding the behavior of the rule itself: - When set to always, the rule checks that the scope is empty [3][4]. - When set to never, the rule checks that the scope is not empty [3][4]. Note that commitlint itself does not ship with a default set of enabled rules; it requires extending a configuration (such as @commitlint/config-conventional) to apply standard conventions [1][5]. You should check your configuration file to see if scope-empty is defined and what its specific severity level (0, 1, or 2) is set to [1][5].
Citations:
- 1: https://github.com/conventional-changelog/commitlint/blob/master/skills/committing-with-commitlint/SKILL.md
- 2: https://commitlint.js.org/support/upgrade.html
- 3: https://commitlint.js.org/reference/rules
- 4: https://raw.githubusercontent.com/conventional-changelog/commitlint/master/docs/reference/rules.md
- 5: https://commitlint.js.org/reference/configuration
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo ".coderabbit.yaml relevant:"
if [ -f .coderabbit.yaml ]; then
awk '{printf "%5d %s\n", NR, $0}' .coderabbit.yaml | sed -n '30,55p'
fi
echo
echo ".commitlintrc.js relevant:"
if [ -f .commitlintrc.js ]; then
awk '{printf "%5d %s\n", NR, $0}' .commitlintrc.js | sed -n '1,80p'
fi
echo
echo "Commitlint related files/workflows:"
git ls-files | grep -E '(^|/)\.commitlintrc\.js$|(^|/)\.coderabbit\.yaml$|^\.github/workflows/.*|package\.json' | sed -n '1,120p'
rg -n "commitlint|commit-lint|CODE_RABBIT|coderabbit|push|pull_request|pr-title|PR_TITLE|scope-empty|type-enum" .github package.json 2>/dev/null | sed -n '1,240p' || trueRepository: linux-system-roles/bootloader
Length of output: 10510
🌐 Web query:
commitlint conventional-changelog parser header scope required if no scope-empty rule
💡 Result:
In commitlint, the scope field is not inherently required by the conventional-changelog parser; its presence or requirement is governed by your commitlint rules, specifically scope-empty [1]. 1. Parser Behavior: The parser is responsible for extracting the scope from the commit header based on the headerPattern [2]. If the pattern does not explicitly make the scope capture group mandatory, the parser will simply return null or undefined for the scope when it is missing [3]. 2. Rule Enforcement: The rule scope-empty controls whether a scope is allowed to be missing [1][4]. - If you set 'scope-empty' to [2, 'never'], a scope becomes mandatory, and commitlint will error if one is not provided [1]. - If you do not have the 'scope-empty' rule enabled (or it is set to severity 0), commitlint will not enforce the presence of a scope, even if the parser correctly identifies it when it is present [1]. 3. Customizing Requirements: If you wish to make the scope mandatory or optional, you should primarily adjust the 'scope-empty' rule in your configuration file rather than relying on the parser alone [3]. If you are using a custom headerPattern, ensure your regex correctly handles the optionality of the scope group (e.g., using (?:\((.*)\))? to make the entire scope segment optional) to prevent the parser from failing to match headers that lack a scope [3].
Citations:
- 1: https://github.com/conventional-changelog/commitlint/blob/master/skills/committing-with-commitlint/SKILL.md
- 2: https://commitlint.js.org/reference/configuration.html
- 3: commit lint issuePrefixes - scope could not be optional conventional-changelog/commitlint#1016
- 4: https://github.com/conventional-changelog/commitlint/blob/master/docs/reference/rules.md
Align CodeRabbit PR-title guidance with Commitlint behavior.
.commitlintrc.js does not define scope-empty, so the PR-title workflow accepts scoped titles such as feat(logging): add JSONL output, which conflicts with the “Format: <type>: <description> or <type>!: <description>” guidance in .coderabbit.yaml.
- If scoped titles should be allowed, update the
.coderabbit.yamlformat/examples to show the Conventional Commits scope form. - If scopes should be prohibited, add
scope-empty: [2, 'always']to.commitlintrc.jsso the workflow rejects scoped titles.
📍 Affects 2 files
.coderabbit.yaml#L42-L43(this comment).commitlintrc.js#L3-L36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.coderabbit.yaml around lines 42 - 43, The commit-title guidance and
validation currently disagree about scoped Conventional Commit titles. Choose
one policy and apply it consistently: either update `.coderabbit.yaml` guidance
and examples to include the optional `(scope)` form, or add the `scope-empty`
rule in `.commitlintrc.js` to reject scopes; modify both named sites as needed
to reflect the chosen behavior.
|
|
||
| - name: checkout PR | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow checkout refs =="
for f in \
.github/workflows/changelog_to_tag.yml \
.github/workflows/codeql.yml \
.github/workflows/codespell.yml \
.github/workflows/markdownlint.yml \
.github/workflows/python-unit-test.yml \
.github/workflows/test_converting_readme.yml
do
if [ -f "$f" ]; then
echo "--- $f"
nl -ba "$f" | sed -n '1,120p'
else
echo "missing $f"
fi
done
echo
echo "== occurrences of persist-credentials =="
rg -n "persist-credentials|git fetch --all --tags|git fetch|codeql|codespell|markdownlint|tox|test_converting_readme|changelog_to_tag" .github/workflows || true
echo
echo "== workflow event/schedule triggers and permissions =="
python3 - <<'PY'
from pathlib import Path
import re, yaml
for p in Path('.github/workflows').glob('*.yml'):
print(f'--- {p}')
text=p.read_text()
m=re.search(r'^(?:on:|permissions:)', text[:200], re.M)
start=0 if not m else m.start()
print('<160 chars:', text[:160])
try:
data=yaml.safe_load(text)
print('top keys:', list(data or {}).keys())
print('on:', data.get('on') or data.get('on'))
print('permissions:', data.get('permissions'))
except Exception as e:
print('yaml parse error:', e)
PYRepository: linux-system-roles/bootloader
Length of output: 280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow checkout refs =="
for f in \
.github/workflows/changelog_to_tag.yml \
.github/workflows/codeql.yml \
.github/workflows/codespell.yml \
.github/workflows/markdownlint.yml \
.github/workflows/python-unit-test.yml \
.github/workflows/test_converting_readme.yml
do
if [ -f "$f" ]; then
echo "--- $f"
awk '{printf "%6d %s\n", NR, $0}' "$f" | sed -n '1,120p'
else
echo "missing $f"
fi
done
echo
echo "== occurrences of persist-credentials and related Git/workflow hooks =="
rg -n "persist-credentials|git fetch --all --tags|git fetch|git clone|git push|git tag|codeql|codespell|markdownlint|tox|test_converting_readme|changelog_to_tag" .github/workflows || true
echo
echo "== workflow event/schedule triggers and permissions =="
python3 - <<'PY'
from pathlib import Path
import re
try:
import yaml
except Exception as e:
print("pyyaml unavailable:", e)
yaml = None
for p in sorted(Path('.github/workflows').glob('*.yml')):
print("---", p)
text = p.read_text()
lines = text.splitlines()
for i, line in enumerate(lines[:80], 1):
if line.startswith(('on:', 'permissions:', 'jobs:')) or re.match(r'^\s*(on|permissions|jobs):', line):
print(f"{i}: {line}")
if yaml is not None:
try:
data = yaml.safe_load(text)
except Exception as e:
print("yaml parse error:", e)
data = None
if data:
print("on:", data.get("on") or data.get("on"))
print("permissions:", data.get("permissions"))
PYRepository: linux-system-roles/bootloader
Length of output: 19631
🌐 Web query:
actions/checkout v6 persist-credentials default token git config
💡 Result:
In actions/checkout v6, the persist-credentials input defaults to true [1][2]. When enabled (the default), the action automatically configures authentication credentials to enable your scripts to run authenticated git commands (such as git fetch or git push) within the workflow [1][3]. A key change in v6 is the improved credential security: instead of writing the auth token directly into the local.git/config file, the action now stores credentials in a separate file located under $RUNNER_TEMP [1][4][5]. Git is then configured to include these credentials via an include or includeIf directive, which keeps the credentials out of the main repository configuration [5][6]. The token is removed during post-job cleanup [1][7]. You can opt out of this behavior by setting persist-credentials to false in your workflow step [1][8]: - uses: actions/checkout@v6 with: persist-credentials: false Running authenticated git commands from a Docker container action requires Actions Runner v2.329.0 or later to properly access these persisted credentials [1][4].
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 2: https://raw.githubusercontent.com/actions/checkout/v6/action.yml
- 3: https://github.com/actions/checkout
- 4: https://github.com/actions/checkout/tree/v6.0.0
- 5: actions/checkout@v5.0.0...v6.0.0
- 6: actions/checkout@v5.0.1...v6.0.0
- 7: https://github.com/actions/checkout/?tab=readme-ov-file
- 8: https://p.rst.im/q/github.com/marketplace/actions/checkout
Disable checkout credential persistence in workflows that do not need it.
actions/checkout@v6 keeps the workflow token available to later Git commands by default. Add persist-credentials: false to the checkout step in:
.github/workflows/changelog_to_tag.yml.github/workflows/codeql.yml.github/workflows/codespell.yml.github/workflows/markdownlint.yml.github/workflows/python-unit-test.yml.github/workflows/test_converting_readme.yml
For changelog_to_tag.yml, use an explicit token for git fetch --all --tags before disabling persistence, because later git fetch steps will run unauthenticated with only contents: read.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 24-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 6 files
.github/workflows/changelog_to_tag.yml#L25-L25(this comment).github/workflows/codeql.yml#L36-L37.github/workflows/codespell.yml#L15-L16.github/workflows/markdownlint.yml#L30-L31.github/workflows/python-unit-test.yml#L45-L46.github/workflows/test_converting_readme.yml#L31-L32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/changelog_to_tag.yml at line 25, Disable checkout
credential persistence by adding persist-credentials: false to the checkout
steps in .github/workflows/changelog_to_tag.yml (lines 25-25),
.github/workflows/codeql.yml (lines 36-37), .github/workflows/codespell.yml
(lines 15-16), .github/workflows/markdownlint.yml (lines 30-31),
.github/workflows/python-unit-test.yml (lines 45-46), and
.github/workflows/test_converting_readme.yml (lines 31-32). In
changelog_to_tag.yml, configure checkout with an explicit token for the later
git fetch --all --tags operation before disabling persistence; leave the other
sites with only the persistence change.
Source: Linters/SAST tools
| - name: Install conventional-commit linter | ||
| run: npm install @commitlint/config-conventional @commitlint/cli |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate workflow and package files =="
git ls-files | rg '(^\.github/workflows/pr-title-lint\.yml$|(^|/)package(-lock)?\.json$|^npm-shrinkwrap\.json$|^pnpm-lock\.yaml$|^yarn\.lock$)' || true
echo
echo "== workflow snippet =="
if [ -f .github/workflows/pr-title-lint.yml ]; then
cat -n .github/workflows/pr-title-lint.yml
fi
echo
echo "== package-related snippets =="
for f in package.json package-lock.json npm-shrinkwrap.json pnpm-lock.yaml yarn.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo
echo "== repo package files summary =="
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').rglob('package.json'):
if any(part.startswith('.') for part in p.parts):
continue
try:
data=json.loads(p.read_text())
except Exception as e:
print(f"{p}: parse error {e}")
continue
print(f"{p}: scripts={data.get('scripts',{})}; deps keys={sorted({**data.get('dependencies',{}), **data.get('devDependencies',{}), **data.get('peerDependencies',{}), **data.get('optionalDependencies',{})}.keys())[:50]}")
PYRepository: linux-system-roles/bootloader
Length of output: 1328
Use locked Commitlint dependencies.
npm install @commitlint/config-conventional @commitlint/cli`` resolves the latest packages on every PR run, and this workflow has no checked-in package.json/lockfile. Add the required packages and lockfile, then use `npm ci`.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 26-26: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-title-lint.yml around lines 25 - 26, Update the
“Install conventional-commit linter” workflow step to use locked dependencies:
add `@commitlint/config-conventional` and `@commitlint/cli` to a checked-in
package.json with its generated lockfile, then replace npm install with npm ci
so PR runs use the locked versions.
Source: Linters/SAST tools
| - name: Checkout repo | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
sed -n '45,140p' .github/workflows/qemu-kvm-integration-tests.yml || true
echo
echo "== checkout usages in workflow =="
rg -n "actions/checkout|persist-credentials|git " .github/workflows/qemu-kvm-integration-tests.yml || true
echo
echo "== docs/local hints in repo =="
rg -n "actions/checkout|persist-credentials" .github README.md 2>/dev/null || trueRepository: linux-system-roles/bootloader
Length of output: 5643
🌐 Web query:
actions/checkout v6 persist-credentials default true README
💡 Result:
The persist-credentials input for actions/checkout v6 has a default value of true [1]. When set to true, the action configures the authentication token or SSH key within the environment to allow subsequent git commands in your workflow to run with authentication [2][1]. In v6, the behavior of this setting was improved for security: the credentials are now stored in a separate file under the $RUNNER_TEMP directory, rather than being written directly to.git/config as they were in previous versions [2][3][4]. The credentials are automatically removed during the post-job cleanup [2][5]. If you wish to disable this persistence, you can explicitly set persist-credentials to false [2][1]. Note that if you are using Docker container actions, this enhanced security feature requires Actions Runner v2.329.0 or later to function correctly [2][3].
Citations:
- 1: https://raw.githubusercontent.com/actions/checkout/v6/action.yml
- 2: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 3: https://github.com/actions/checkout/tree/v6.0.0
- 4: actions/checkout@v5.0.1...v6.0.0
- 5: https://github.com/actions/checkout/?tab=readme-ov-file
Disable persisted checkout credentials.
This workflow installs and runs test dependencies after checkout, but no later step needs authenticated Git access. Set persist-credentials: false to avoid leaving checkout credentials available for the rest of the job.
Proposed fix
- name: Checkout repo
uses: actions/checkout@v6
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repo | |
| uses: actions/checkout@v7 | |
| uses: actions/checkout@v6 | |
| - name: Checkout repo | |
| uses: actions/checkout@v6 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 53-54: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/qemu-kvm-integration-tests.yml around lines 53 - 54,
Update the actions/checkout step in the workflow to set persist-credentials to
false, ensuring checkout credentials are not retained for subsequent test
dependency and execution steps.
Source: Linters/SAST tools
| import tempfile | ||
| import unittest | ||
|
|
||
| import sr_fingerprint |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for test path configuration that exposes the library directory to unit tests.
fd -t f -g 'conftest.py'
fd -t f -g 'tox.ini' -g 'pytest.ini' -g 'setup.cfg' -g 'pyproject.toml'
rg -n 'library' --glob 'tox.ini' --glob 'pytest.ini' --glob 'setup.cfg' --glob 'pyproject.toml' --glob 'conftest.py'
# Confirm how sibling unit tests import modules under library/.
fd -t f -e py . tests/unit --exec rg -n '^import |^from ' {}Repository: linux-system-roles/bootloader
Length of output: 448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== root files/directories relevant to tests/config =="
git ls-files | rg '(^|/)(conftest\.py|tox\.ini|pytest\.ini|setup\.cfg|pyproject\.toml)$|(^|/)tests/|^library/' | head -200
echo
echo "== found conftest files =="
git ls-files | rg '/conftest\.py$' || true
echo
echo "== test config files occurrences =="
git ls-files | rg '(tox\.ini|pytest\.ini|setup\.cfg|pyproject\.toml)$' || true
for f in $(git ls-files | rg '(tox\.ini|pytest\.ini|setup\.cfg|pyproject\.toml)$' || true); do
echo "-- $f"
sed -n '1,220p' "$f"
done
echo
echo ==" search library references in repo config/test files =="
rg -n 'library|src_paths|pythonpath|sys\.path|importlib|molecule|ansible' --glob 'tox.ini' --glob 'pytest.ini' --glob 'setup.cfg' --glob 'pyproject.toml' --glob '**/conftest.py' . || true
echo
echo "== unit test import lines =="
git ls-files 'tests/**/*.py' | xargs -r rg -n '^import |^from ' || trueRepository: linux-system-roles/bootloader
Length of output: 2427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tox lsr tools availability =="
python3 - <<'PY'
import shutil
for name in ["tox", "python", "pip"]:
print(name, "=", shutil.which(name))
PY
echo
echo "== tox -e black,flake8 environment summary =="
tox -e black,flake8 -a 2>/tmp/tox-list.log || { cat /tmp/tox-list.log; exit 0; }
echo
echo "== exact tox lines for black and flake8 =="
sed -n '1,220p' tox.iniRepository: linux-system-roles/bootloader
Length of output: 340
Make the module import explicit for unit testing.
tests/unit/test_sr_fingerprint.py imports sr_fingerprint, but the repository configuration does not add library/ to sys.path. Add library to the tox test configuration or use a package-relative import so collection cannot fail on import path setup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_sr_fingerprint.py` at line 16, Update the import setup for
tests/unit/test_sr_fingerprint.py so sr_fingerprint resolves reliably during
test collection: either add library to the tox test configuration path or
replace the import with the appropriate package-relative form. Preserve the
existing test behavior and ensure collection does not depend on an implicit
sys.path entry.
4c047a3 to
8afd2e1
Compare
| for path in (log_file, log_file + ".lock"): | ||
| try: | ||
| os.unlink(path) | ||
| except OSError: |
8afd2e1 to
7823667
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/unit/test_sr_fingerprint.py (1)
346-348: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the complete timestamp format.
This assertion accepts any dot-free string. Verify the date, time, seconds, and UTC offset so the test enforces
_local_iso8601_no_microsecondsoutput contract.Proposed fix
def test_local_iso8601_no_microseconds_has_no_fraction(self): timestamp = sr_fingerprint._local_iso8601_no_microseconds() - self.assertNotIn(".", timestamp) + self.assertRegex( + timestamp, + r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{2}:?\d{2}$", + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_sr_fingerprint.py` around lines 346 - 348, Strengthen test_local_iso8601_no_microseconds_has_no_fraction to assert the complete _local_iso8601_no_microseconds output format, including date, time, seconds, and UTC offset, rather than only checking that the timestamp lacks a period. Use a format-aware assertion that also preserves the no-microseconds requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@library/sr_fingerprint.py`:
- Around line 219-231: Update _write_jsonl_log so a positive max_size smaller
than new_line is handled explicitly before any _trim_log_file call. Reject the
undersized limit with a clear error message, and ensure trimming is only
attempted when an existing log file can be trimmed, including the additional
call site noted in the review.
- Around line 191-196: In _trim_log_file, rename the sum-comprehension variable
l to line to satisfy Ruff E741, preserving the existing trimming behavior.
Format the file with Black and run tox -e black,flake8 before committing.
- Around line 222-233: The _write_jsonl_log flow must reject symlinks for both
log_file and its “.lock” sidecar before writing. Replace builtin open calls with
os.open using O_NOFOLLOW and suitable write/create flags, verify each opened
descriptor refers to a regular file via fstat and S_ISREG, then use the
validated descriptors for locking and appending while preserving existing
trimming behavior.
In `@tests/unit/test_sr_fingerprint.py`:
- Around line 130-136: Reformat the self.assertIn call in the
test_format_fingerprint_syslog_quotes_values_with_spaces method to comply with
Black's line length limits. Wrap the assertion arguments so that no single line
exceeds Black's default line length threshold while preserving the test logic
and the expected substring being checked.
---
Nitpick comments:
In `@tests/unit/test_sr_fingerprint.py`:
- Around line 346-348: Strengthen
test_local_iso8601_no_microseconds_has_no_fraction to assert the complete
_local_iso8601_no_microseconds output format, including date, time, seconds, and
UTC offset, rather than only checking that the timestamp lacks a period. Use a
format-aware assertion that also preserves the no-microseconds requirement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: be95ea67-ddce-4fd7-9bb5-4e334817e60e
📒 Files selected for processing (19)
.coderabbit.yaml.commitlintrc.js.github/workflows/ansible-lint.yml.github/workflows/ansible-managed-var-comment.yml.github/workflows/ansible-test.yml.github/workflows/build_docs.yml.github/workflows/changelog_to_tag.yml.github/workflows/codeql.yml.github/workflows/codespell.yml.github/workflows/markdownlint.yml.github/workflows/pr-title-lint.yml.github/workflows/python-unit-test.yml.github/workflows/qemu-kvm-integration-tests.yml.github/workflows/test_converting_readme.yml.github/workflows/tft.yml.github/workflows/weekly_ci.yml.github/workflows/woke.ymllibrary/sr_fingerprint.pytests/unit/test_sr_fingerprint.py
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/weekly_ci.yml
- .github/workflows/ansible-managed-var-comment.yml
- .github/workflows/build_docs.yml
- .coderabbit.yaml
- .github/workflows/markdownlint.yml
- .commitlintrc.js
- .github/workflows/ansible-test.yml
| os.fchmod(fd, stat.S_IMODE(orig_stat.st_mode)) | ||
| try: | ||
| os.fchown(fd, orig_stat.st_uid, orig_stat.st_gid) | ||
| except OSError: |
| except BaseException: | ||
| try: | ||
| os.unlink(tmp_path) | ||
| except OSError: |
5b613c3 to
d3029de
Compare
|
[citest] |
d3029de to
6b0a18d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
a6a7b5c to
5cb6417
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@library/sr_fingerprint.py`:
- Around line 231-232: Update the size-handling logic around _trim_log_file to
trim the full overflow, calculated as cur_size plus len(new_line) minus
max_size, whenever the write would exceed max_size, including files already over
the limit. Define and test the expected behavior for a single JSONL record
larger than max_log_size, preserving valid record handling.
- Around line 283-287: Update _format_fingerprint_key_value to sanitize
carriage-return and newline characters in field values before formatting them
for module.log, preventing multiline syslog entries while preserving existing
quoting behavior. Add a regression test covering a distribution value containing
a newline and verify the emitted fingerprint remains a single log entry.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e9f99ae-0d34-481f-ade1-4e11dfeeb35d
📒 Files selected for processing (5)
library/sr_fingerprint.pytasks/main.ymltasks/set_vars.ymltests/tests_default.ymltests/unit/test_sr_fingerprint.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tasks/set_vars.yml
- tasks/main.yml
- tests/tests_default.yml
| if max_size > 0 and cur_size + len(new_line) > max_size and cur_size > 0: | ||
| _trim_log_file(log_file, len(new_line)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Enforce max_log_size after a limit change.
If an existing file is already larger than max_log_size, this call removes only len(new_line) bytes. The file can remain above the configured limit after every later write.
Calculate cur_size + len(new_line) - max_log_size and trim that overflow. Define and test the behavior when one JSONL record is larger than max_log_size.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 232-232: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(log_file, "a")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@library/sr_fingerprint.py` around lines 231 - 232, Update the size-handling
logic around _trim_log_file to trim the full overflow, calculated as cur_size
plus len(new_line) minus max_size, whenever the write would exceed max_size,
including files already over the limit. Define and test the expected behavior
for a single JSONL record larger than max_log_size, preserving valid record
handling.
| def _format_fingerprint_key_value(field, value): | ||
| text = "" if value is None else str(value) | ||
| if any(char in text for char in ' "='): | ||
| return '%s="%s"' % (field, text.replace('"', '""')) | ||
| return "%s=%s" % (field, text) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file outline =="
ast-grep outline library/sr_fingerprint.py --view expanded | sed -n '1,220p' || true
echo "== sr_fingerprint relevant sections =="
cat -n library/sr_fingerprint.py | sed -n '1,340p'
echo "== imports/usages of module.log in sr_fingerprint =="
rg -n "module\.log|log_file|write_log|fingerprint|_format_fingerprint_syslog|_format_fingerprint_jsonl|managed_node_distro|distribution" library/sr_fingerprint.py
echo "== tests relevant sections =="
cat -n tests/unit/test_sr_fingerprint.py | sed -n '1,170p'
cat -n tests/unit/test_sr_fingerprint.py | sed -n '200,370p'
echo "== deterministic behavior probe for sanitizer =="
python3 - <<'PY'
def _format_fingerprint_key_value(field, value):
text = "" if value is None else str(value)
if any(char in text for char in ' "='):
return '%s="%s"' % (field, text.replace('"', '""'))
return "%s=%s" % (field, text)
for v in ["RedHat\nLinux", "RedHat\rLinux", '"a\nb"']:
print(repr(v), "=>", repr(_format_fingerprint_key_value("distribution", v)))
PYRepository: linux-system-roles/bootloader
Length of output: 32975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all module.log occurrences in repository =="
rg -n "module\.log\(" .
echo "== module.log definitions/usages in collection roles (if present) =="
rg -n "def log\b|module\.log|syslog|rsyslog|journalctl|/var/log|ansible.builtin.logging|ansible.builtin.syslog" .Repository: linux-system-roles/bootloader
Length of output: 2178
🌐 Web query:
Ansible module_utils basic module.log syslog message newline escape control characters
💡 Result:
The Ansible module.log() method, defined within ansible.module_utils.basic.AnsibleModule, is designed to send log messages to system logging facilities (syslog or systemd-journal) rather than standard output or error [1][2][3]. Regarding the handling of newline characters and escape sequences: 1. Escape Character Handling: The log() method does not perform explicit escaping of control characters such as newlines (\n) before sending the string to the logging system [1][2]. Because it ultimately relies on the underlying system's syslog or journald implementation, messages containing literal newline characters can break the log format, causing the log entry to span multiple lines in the target file [1][4]. 2. Potential Errors: The inclusion of certain control characters, particularly null characters (\0), can cause the log() function to fail. Historical issues have identified that passing strings containing embedded null characters results in a ValueError during the logging process, causing module execution to crash [4]. 3. Recommendations: If you need to log messages that contain newlines or control characters, it is best practice to sanitize or replace these characters manually within your module code before passing the string to module.log() [1][2]. You can use standard Python string methods like .replace('\n', ' ') or encoding/escaping functions to ensure the log output remains stable and readable within the system logs [4]. Because module.log() is intended for internal tracking and not for direct user feedback, output is not visible via standard Ansible verbosity (e.g., -v to -vvvv), which is why standard print() statements or specific error reporting methods are generally preferred for troubleshooting or user-facing output [3].
Citations:
- 1: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/basic.py
- 2: https://github.com/ansible/ansible/blob/33ee5d4c1b8ea5357e29749a71f55d8f03642b9e/lib/ansible/module_utils/basic.py
- 3: https://stackoverflow.com/questions/55117609/ansible-ansiblemodule-log-method-does-not-print-anything-to-stdout-stderr
- 4: syslog: Handle ValueError raised while sending logs to syslog ansible/ansible#82225
Other (CWE-117)
Sanitize syslog values before module.log.
_format_fingerprint_key_value preserves \n and \r in values like distribution, and module.log() passes that message to syslog/journald without newline escaping. This can split a single fingerprint into multiple logical log entries. Escape or reject CR/LF before module.log() and add a regression test with a newline in distribution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@library/sr_fingerprint.py` around lines 283 - 287, Update
_format_fingerprint_key_value to sanitize carriage-return and newline characters
in field values before formatting them for module.log, preventing multiline
syslog entries while preserving existing quoting behavior. Add a regression test
covering a distribution value containing a newline and verify the emitted
fingerprint remains a single log entry.
|
|
||
| def test_local_iso8601_no_microseconds_has_no_fraction(self): | ||
| timestamp = sr_fingerprint._local_iso8601_no_microseconds() | ||
| self.assertRegex( |
There was a problem hiding this comment.
unfortunately python 2.7 unittest has no assertRegex method - so you'll have to use re.match and then assertTrue(match_result is not None) or something like that
"syslog" not "rsyslog" Also - we still need a way to test this upstream - the role has
"syslog" not "rsyslog" |
be79445 to
17a98c5
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_sr_fingerprint.py`:
- Around line 79-82: Wrap the long expected syslog string in the relevant
fingerprint test by splitting the adjacent string literals so each line complies
with Black and PEP 8; preserve the exact concatenated expected value, then run
tox -e black,flake8.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c3a0a87-0b9c-4927-96bf-8e3339a11252
📒 Files selected for processing (2)
library/sr_fingerprint.pytests/unit/test_sr_fingerprint.py
|
[citest] |
f421469 to
43cb8a0
Compare
43cb8a0 to
0050eaf
Compare
|
[citest] |
|
[citest] |
spetrosi
changed the title
0050eaf to
890c25c
Compare
Feature: Write roles fingerprints to /var/log/sysroles.jsonl [citest_skip] Reason: By default logs are printed to rsyslog. This change adds a possibility to write logs to a file on the system for the downstream users. Result: For the upstream, this makes rsyslog log message more detailed. For the downstream - also writes logs to /var/log/sysroles.jsonl Signed-off-by: Sergei Petrosian <spetrosi@redhat.com>
The sr_fingerprint module was rewritten to accept structured parameters (status, role_name, role_path, etc.) instead of a free-form sr_message. Update the role tasks and tests to match the new module interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
890c25c to
8921bf1
Compare
The sr_fingerprint module was rewritten to accept structured parameters (status, role_name, role_path, etc.) instead of a free-form sr_message. Update the role tasks and tests to match the new module interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[citest] |
3 participants
Feature: Write roles fingerprints to /var/log/sysroles.jsonl [citest_skip]
Reason: By default logs are printed to rsyslog. This change adds a possibility to write logs to a file on the system for the downstream users.
Result: For the upstream, this makes rsyslog log message more detailed. For the downstream - also writes logs to /var/log/sysroles.jsonl