Skip to content

Give ro_strict kibana access real e2e coverage #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sscarduzio merged 1 commit into from
Sep 13, 2026
Merged

Give ro_strict kibana access real e2e coverage #130

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2e-tests/cypress/fixtures/roStrictSettings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
helpers:
cr: &common-rules
kibana_access: ro
kibana_access: ro_strict

@10hexdev 10hexdev Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ro_strict spec runs the same read-only assertions as ro, so the one behavior that separates the two levels is never exercised.

$ grep -rn "clickCopyLinkButton" e2e-tests/cypress/e2e e2e-tests/cypress/support
RoAndRoStrictKibanaAccessAssertions.ts:72:  Discover.clickCopyLinkButton('ro')

Discover.clickCopyLinkButton (Discover.ts:116-126) creates a short URL only for admin and rw. For 'ro' (and for 'ro_strict') it clicks the copy button and does nothing else. No POST /api/short_url, no _update/index-pattern*, no config/*/_create, no doc/telemetry* — none of the writes in BaseKibanaRule.scala:278-279 that ro_strict refuses.

The two specs are the same test with a different fixture name; after substituting the fixture name they are byte-identical. So this spec passes under ro_strict and under ro, and it would also pass if the plugin silently downgraded ro_strict to ro. It proves "read-heavy pages render under ro_strict" — fail-closed is covered. It cannot prove "ro_strict refuses writes" — fail-open is not covered.

Action: add one assertion that performs a write the non-strict list allows and asserts it is refused — drive Discover.clickCopyLinkButton('ro_strict') through a branch that intercepts POST /api/short_url and asserts 403, or a Direct-kibana-request.cy.ts-style call under the ro_strict fixture asserting the ROR forbidden envelope for _update/index-pattern:*. If that write is intentionally out of scope, rename the spec so it no longer claims ro_strict coverage as a whole, and state which behavior it does cover.

Severity: decide.

@10hexdev 10hexdev Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same one-line bug is live in the Kibana plugin repo, and nothing tracks the companion fix.

readonlyrest_kbn/automatic-tests/cypress/fixtures/roStrictSettings.yaml has kibana_access: ro on line 3 — the PR body confirms diff -w against its roSettings.yaml is empty. The two repos sync these fixtures (RoAndRoStrictKibanaAccessAssertions.ts:31-32). This PR is now merged, so the next sync can silently restore ro here or never deliver ro_strict there.

Action: open the matching readonlyrest_kbn PR now and link it in this thread, or file a tracking issue pointing at this PR — so the fix does not depend on memory.

Severity: decide.

kibana_hide_apps: ["Enterprise Search|Overview", "Observability"]
kibana_index: ".kibana_@{acl:current_group}"

Expand Down
Loading