feat(garm): OWASP security event logging — Stage 1 producers (ISD-6341) by florentianayuwono · Pull Request #408 · canonical/github-runner-operators

florentianayuwono · 2026-10-06T18:14:41Z

What this PR does

Implements Stage-1 (§25.10) of OWASP-framework security event logging for the garm charm — the log producer side. Instruments the charm to emit structured OWASP security events (via the owasp-logger library, nested under an owasp_event key) for the priority event classes:

AUTHN — authn_login_success, authn_login_fail, authn_token_created, authn_token_revoked, authn_token_delete
AUTHZ — authz_fail (GARM 401, emitted from every privileged mutation path), authz_admin (privileged resource mutations: org/repo/scaleset/template CRUD, get-credentials action)
SYS — sys_startup, sys_shutdown
USER — user_created (GARM first-run)

Forwarding. Workload-stdout events are forwarded to Loki via the go-framework extension's built-in LogForwarder (no extra logging relation needed). Charm-hook events are written to juju-log, which Juju forwards to the Juju controller — their onward handling and retention is therefore the Juju controller operator's responsibility, not this charm's. (An earlier Stage-3 proposal to re-forward charm-hook events over OTLP from the charm has been dropped and its PR closed.)

Also adds a How-to guide for granting read-only, least-privilege access to the security feed (docs/how-to/access-security-logs.md).

Why we need it

Fulfils the ISD-6341 requirement to emit a standardized, auditable security-event trail for the portions of GARM that Canonical is responsible for, enabling independent security auditing with least-privilege log access.

Test plan

All run from a clean checkout of this branch:

cd charms/garm && tox -c tox.toml -e fmt,lint,complexity,static,unit → passed (435 unit tests, all gates green)
cd docs && make html (Sphinx --fail-on-warning) → build succeeded
make -C docs spellcheck → passed (0 errors, 16 files)

Review focus

src/garm_api.py carries the bulk of the instrumentation; authz_admin spans the privileged mutation paths, and authz_fail is now emitted on a GARM 401 from every privileged mutation (via the shared _raise_api_error / _log_authz_fail_on_401 helpers) rather than only the scaleset/instance paths.
The charm deliberately does not declare a second logging relation / LogForwarder — the go-framework extension already provides one (declaring it again errors on overlapping keys).
Event token formats follow owasp-logger's <name>:<comma-args> convention exactly. Note authn_token_delete's positional arg is the deletion target (forge-credential-<cred_id>), not the global appid.
Docs: the guide lives under the How-to Diátaxis section (not a bespoke "Security" toctree caption), per docs-review feedback.

Potential breaking changes

None. Purely additive logging; no schema or relation changes.

New dependencies

Adds owasp-logger to the charm's requirements.txt.

Checklist

Part 1 of 2 (stacked). Stage-2 (Sigma detection rules, #409) is stacked on top of this branch. Tracking: ISD-6341.

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved issues affect authentication auditing, actor identity, Loki forwarding, documented queries, and the required changelog entry.

Review effort: Lite
Findings: 3 · 1

Open (4)

Emit authentication failure audit event before raising on empty token · New
Forward hook-generated security events to Loki · New
Correct Loki query to filter nested appid field · New
Add documentation change to the changelog · New

What changed in this PR

Adds Stage 1 OWASP security-event logging to the garm charm, with API instrumentation, tests, dependency updates, and security-log documentation.

Changes:

Instruments authentication, authorization, lifecycle, and administrative events.
Adds the owasp-logger dependency and unit coverage.
Documents Security Logs Viewer access and Loki filtering.

File	Summary
`docs/security/README-security-logs-viewer.md`	Documents security-log access and filtering.
`docs/.custom_wordlist.txt`	Adds documentation vocabulary.
`charms/garm/tests/unit/test_security_log.py`	Tests structured event output.
`charms/garm/tests/unit/test_garm_api.py`	Tests API event emission.
`charms/garm/tests/unit/test_charm.py`	Tests lifecycle and action events.
`charms/garm/src/security_log.py`	Defines the OWASP logger configuration.
`charms/garm/src/garm_api.py`	Emits API security events.
`charms/garm/src/charm.py`	Emits lifecycle and user events.
`charms/garm/requirements.txt`	Adds `owasp-logger`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

Unresolved moderate issues remain in 401 event coverage and HTTPS OTLP forwarding.

2 open findings

GARM 401 errors missing authz_fail events · New
HTTPS collector endpoints use insecure gRPC · New

4 resolved since last review

Correct Loki query to filter nested appid field
Forward hook-generated security events to Loki
Emit authentication failure audit event before raising on empty token
Add documentation change to the changelog

🧠 Review effort: Lite

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…admin Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…o Loki The go-framework charmcraft extension already provides the 'logging' relation and instantiates a loki_push_api LogForwarder, so the charm relies on that rather than declaring a conflicting second forwarder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…titlement typing - Guard forge-credential name against None for authn_token_created entitlements. - Add a parametrized test asserting every privileged GARM mutation emits authz_admin. - Apply ruff formatting to the new instrumentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add the RBAC Security Logs Viewer role documentation and the Vale wordlist terms it relies on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Point the Security Logs Viewer forwarding section at the concrete mechanisms already in the repo: the go-framework LogForwarder plus the OTEL_LOGS_EXPORTER=console stdout path the sibling 12-factor charms use, and the enable-log-forwarding action's filelog-receiver-to-OTLP pattern as a reference for capturing charm-hook events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The upstream owasp-logger renders the event timestamp in the host's local UTC offset (via .astimezone()). OWASP requires UTC-aligned timestamps, so subclass the logger to emit RFC3339 UTC ('...Z') regardless of host timezone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Emit authn_login_fail before raising on an empty login token, so that path leaves an audit record. - Attribute forge-credential deletions to the credential id subject. - Replace the literal garm-admin actor with a documented _GARM_ADMIN_ACTOR constant explaining the single built-in admin account. - Fix the Loki queries to parse the nested owasp_event JSON, drop the internal SSDLC reference, condense the forwarding section, and add a changelog entry. - Remove the divider comment in the charm tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

authz_fail was only emitted from _raise_resource_api_error, which the scaleset and instance paths use; the organization, repository, template, credential, scaleset-create, controller, and agent-tool mutations catch ApiException directly, so a GARM 401 there raised without an AUTHZ event. Add _raise_api_error, which logs authz_fail on a 401 and then raises the flat GarmApiError (preserving the existing exception type so callers that special-case GarmNotFoundError/GarmUnauthorizedError are unaffected), and route every direct-catch privileged mutation through it. Centralize the single authz_fail emission point via _log_authz_fail_on_401, shared with _raise_resource_api_error. Also add the Security Logs Viewer guide to the docs toctree so the Sphinx (RTD) build no longer fails on the "not included in any toctree" warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…oller Charm-hook security events are written to juju-log and forwarded to the Juju controller, so their onward handling and retention is the Juju controller operator's responsibility rather than this charm's. Drop the OTLP-collector forwarding recommendation and document controller-side inspection via `juju debug-log`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The dedicated "Security" toctree caption was only a workaround for the "document not included in any toctree" build error. Per Diátaxis, this is a how-to guide, so relocate it to docs/how-to/ (anchor how_to_access_security_logs), list it in the How-to toctree and the Operations overview row, and drop the Security caption. Addresses docs review feedback on #408. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

erinecon

Thanks for clarifying the role of the documentation in this PR! The page currently has some tension between how it's written (Reference-like) and where it's placed (How-to guide). I agree the information is useful, but either the page needs a major refactor, or the page needs to be reorganized under Reference

Co-authored-by: Erin Conley <erin.conley@canonical.com>

…ity-logging-stage1-isd-6341 # Conflicts: # docs/how-to/index.rst

Address erinecon's review: the page described the security event feed rather than instructing an operator task, so relocate it from How-to to Reference. - New docs/reference/security-logs.md with descriptive (reference-tone) headings - Link it from the home-page Security row, the Reference toctree, and reference/security.md "Related information" - Remove docs/how-to/access-security-logs.md and its home-page Operations link - Apply grammar suggestions (Charmcraft capitalization, relation wording) - Alphabetize this PR's additions to docs/.custom_wordlist.txt - Update changelog to point at the new reference path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…-stage1-isd-6341' into feat/garm-owasp-security-logging-stage1-isd-6341 # Conflicts: # docs/how-to/access-security-logs.md

Sort the whole wordlist case-insensitively (regex-form entries sort last) so new terms have an obvious insertion point, per review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The newer unpinned pyright (garm) and mypy 2.4.0 (garm-configurator) no longer narrow the IPv4/IPv6 union on a `.version` guard, so the subsequent `start <= end` comparison is flagged as an unsupported operand across address versions. Compare the integer forms instead; for same-version addresses the integer ordering is identical, so behaviour is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

erinecon

Thanks for the quick turnaround! Nits only on the docs side

Co-authored-by: Erin Conley <erin.conley@canonical.com>

florentianayuwono mentioned this pull request Oct 6, 2026

feat(security): OWASP Stage 2 — GARM Sigma detection rules (ISD-6341) #409

Merged

5 of 12 tasks

florentianayuwono added this pull request to stack #410 October 6, 2026 18:17

florentianayuwono self-assigned this Oct 6, 2026

florentianayuwono requested a lite review from Copilot October 6, 2026 18:34

Copilot started reviewing on behalf of florentianayuwono October 6, 2026 18:34

View session

florentianayuwono force-pushed the feat/garm-owasp-security-logging-stage1-isd-6341 branch from fc7ae10 to e6fb3c0 Compare October 6, 2026 18:35

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread charms/garm/src/garm_api.py

Comment thread charms/garm/src/security_log.py Outdated

Comment thread docs/security/README-security-logs-viewer.md Outdated

Comment thread docs/security/README-security-logs-viewer.md Outdated

florentianayuwono commented Oct 7, 2026

View reviewed changes

Comment thread docs/security/README-security-logs-viewer.md Outdated

Comment thread charms/garm/requirements.txt

Comment thread docs/security/README-security-logs-viewer.md Outdated

florentianayuwono commented Oct 7, 2026

View reviewed changes

Comment thread charms/garm/src/garm_api.py Outdated

Comment thread charms/garm/src/garm_api.py Outdated

Comment thread charms/garm/tests/unit/test_charm.py Outdated

florentianayuwono commented Oct 7, 2026

View reviewed changes

Comment thread docs/security/README-security-logs-viewer.md Outdated

florentianayuwono requested a lite review from Copilot October 8, 2026 04:56

Copilot started reviewing on behalf of florentianayuwono October 8, 2026 04:56

View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread charms/garm/src/garm_api.py Outdated

Comment thread charms/garm/src/security_log.py Outdated

florentianayuwono force-pushed the feat/garm-owasp-security-logging-stage1-isd-6341 branch from 87406a0 to 86e7fac Compare October 8, 2026 14:22

florentianayuwono mentioned this pull request Oct 8, 2026

feat(security): OWASP Stage 3 — forward GARM charm-hook events over OTLP (ISD-6341) #416

Closed

5 of 6 tasks

florentianayuwono and others added 14 commits October 8, 2026 22:46

feat(garm): add owasp-logger security_log singleton

ecc3cda

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit user_created on GARM first-run

c0365d2

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit authn login success/fail/token-created on login

ea79976

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit authz_fail on GARM 401 responses

caa4859

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit token lifecycle events on credential CRUD

85d4e26

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit authz_admin on privileged GARM resource mutations

453143e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

feat(garm): emit sys_startup, sys_shutdown and get-credentials authz_…

2a099e6

…admin Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

docs(security): document the Security Logs Viewer read-only role

ab788a0

Add the RBAC Security Logs Viewer role documentation and the Vale wordlist terms it relies on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

florentianayuwono force-pushed the feat/garm-owasp-security-logging-stage1-isd-6341 branch from 86e7fac to c1ea46a Compare October 8, 2026 16:18

florentianayuwono marked this pull request as ready for review October 8, 2026 16:38

florentianayuwono requested review from cbartz, javierdelapuente, weiiwang01, yanksyoon and yhaliaw as code owners October 8, 2026 16:38

florentianayuwono requested a review from erinecon October 9, 2026 03:40

erinecon reviewed Oct 9, 2026

View reviewed changes

Comment thread docs/index.rst Outdated

erinecon reviewed Oct 9, 2026

View reviewed changes

Comment thread docs/how-to/access-security-logs.md Outdated

florentianayuwono and others added 2 commits October 9, 2026 22:41

florentianayuwono requested a review from erinecon October 9, 2026 16:12

yanksyoon approved these changes Oct 13, 2026

View reviewed changes

erinecon requested changes Oct 13, 2026

View reviewed changes

florentianayuwono and others added 6 commits October 13, 2026 22:16

Update docs/how-to/access-security-logs.md

d9c6fe0

Co-authored-by: Erin Conley <erin.conley@canonical.com>

Update docs/how-to/access-security-logs.md

f8c7e59

Co-authored-by: Erin Conley <erin.conley@canonical.com>

Merge remote-tracking branch 'origin/main' into feat/garm-owasp-secur…

851827c

…ity-logging-stage1-isd-6341 # Conflicts: # docs/how-to/index.rst

Merge remote-tracking branch 'origin/feat/garm-owasp-security-logging…

07063e9

…-stage1-isd-6341' into feat/garm-owasp-security-logging-stage1-isd-6341 # Conflicts: # docs/how-to/access-security-logs.md

docs: alphabetize docs/.custom_wordlist.txt

4c86bcb

Sort the whole wordlist case-insensitively (regex-form entries sort last) so new terms have an obvious insertion point, per review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

florentianayuwono requested a review from erinecon October 13, 2026 17:44

erinecon approved these changes Oct 13, 2026

View reviewed changes

Comment thread docs/reference/security-logs.md Outdated

Comment thread docs/reference/security-logs.md Outdated

Comment thread docs/how-to/access-security-logs.md Outdated

florentianayuwono and others added 2 commits October 14, 2026 09:57

Update docs/reference/security-logs.md

3c17f2f

Co-authored-by: Erin Conley <erin.conley@canonical.com>

Update docs/reference/security-logs.md

3940549

Co-authored-by: Erin Conley <erin.conley@canonical.com>

florentianayuwono added a commit that referenced this pull request Oct 14, 2026

Merge updated stage1 base (#408) into stage2

965ee52

florentianayuwono merged commit 81b483e into main Oct 14, 2026
62 checks passed

florentianayuwono deleted the feat/garm-owasp-security-logging-stage1-isd-6341 branch October 14, 2026 04:26

Conversation

florentianayuwono commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why we need it

Test plan

Review focus

Potential breaking changes

New dependencies

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erinecon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erinecon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

florentianayuwono commented Oct 6, 2026 •

edited

Loading