docs: document confluence.spaces allowlist in config/README.md [doc-updater] - #2149
Conversation
Add confluence.spaces allowlist section to the context-filters.yaml documentation in config/README.md, mirroring the existing jira.projects section. The confluence gateway wrapper (PR #2141) added this configuration key but the README only covered the jira.projects allowlist. Authored-by: egg
There was a problem hiding this comment.
Review of #2149
Documentation-only change to config/README.md adding a confluence.spaces section that mirrors the existing jira.projects section. The intent is sound and the structure parallels the Jira docs well, but the new section misrepresents the actual code behavior in two places. Since the entire purpose of the PR is to document existing behavior accurately, these are blocking.
Blocking
1. Regex shown does not match the implementation.
The PR documents the space-key format as:
Keys must match Atlassian space key format: leading letter followed by letters, digits, or underscores (
[A-Z][A-Z0-9_]*). Case-sensitive.
The actual regex in gateway/confluence_policy.py:65 is:
_SPACE_KEY_RE = re.compile(r"^[a-zA-Z][a-zA-Z0-9_]*$")This accepts both upper- and lowercase letters, unlike the Jira regex (^[A-Z][A-Z0-9_]*$ at gateway/jira_policy.py:62) which is uppercase-only. The inline comment on the actual confluence: block in config/context-filters.yaml correctly says "leading letter followed by letters / digits / underscore" without the uppercase qualifier — which is why this section was deliberately distinguished from the Jira one.
The README's prose says "leading letter" (correct) but then contradicts itself with the regex [A-Z][A-Z0-9_]* (incorrect). This was almost certainly copy-pasted from the Jira section without verifying. An operator reading this will believe docs is invalid where the code actually accepts it.
Fix: change the regex to [a-zA-Z][a-zA-Z0-9_]* to match the implementation.
2. "Case-sensitive" is misleading without context.
The matching is case-sensitive (so eng won't match ENG), but pairing "case-sensitive" with the wrong uppercase-only regex compounds the confusion — readers will think the format itself is uppercase-only. Consider rephrasing alongside the regex fix, e.g.:
Keys must match the Atlassian space-key shape: leading letter followed by letters, digits, or underscores (
[a-zA-Z][a-zA-Z0-9_]*). Lookups are case-sensitive (e.g.,ENG≠eng).
Non-blocking
3. Missing "Invalid entries are logged and ignored" note.
The Jira section includes this clause; the new Confluence section does not. The behavior is identical — confluence_policy.py:178 logs "Ignoring invalid Confluence space key in confluence.spaces" for mismatching entries. Add the clause for symmetry with the Jira section.
4. Endpoint enumeration is slightly imprecise.
all page/search/execute requests return 403
The actual surface includes page/* (get, descendants, footer-comments, inline-comments), space/* (pages, list), search, and execute. "page" reasonably covers page/*, but space enumeration via space/list isn't obviously implied. Consider "all page, space, search, and execute requests return 403" for accuracy.
Verified
- Fail-closed semantics on missing file / section / malformed YAML (
confluence_policy.py:122-146). - Hot-reload via
POST /api/v1/config/reloadand SIGHUP both callreload_confluence_policy(gateway.py:280, 985). - Empty-list semantics (
confluence.spaces: []→ emptyfrozenset→ all calls 403). - CQL scope-extractor pointer resolves correctly to
docs/reference/confluence-wrapper.md.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address reviewer feedback on PR #2149: - Correct the regex from `[A-Z][A-Z0-9_]*` to `[a-zA-Z][a-zA-Z0-9_]*` to match `_SPACE_KEY_RE` in gateway/confluence_policy.py:65 (the Confluence policy accepts mixed-case keys, unlike Jira). - Reword the case-sensitivity note with a concrete example so it does not compound with the (now corrected) regex. - Add the 'Invalid entries are logged and ignored' clause for symmetry with the jira.projects entry; behavior matches confluence_policy.py:178. - Broaden the empty-list 403 enumeration from 'page/search/execute' to 'page, space, search, and execute' so space/list is covered. Authored-by: egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review of #2149
All four points from the prior review have been correctly addressed in f20a09b:
- Regex fixed: now
[a-zA-Z][a-zA-Z0-9_]*, matchinggateway/confluence_policy.py:65(_SPACE_KEY_RE = re.compile(r"^[a-zA-Z][a-zA-Z0-9_]*$")). - Case-sensitivity rephrased: "Lookups are case-sensitive (e.g.,
ENG≠eng)" — no longer reads as "format is uppercase only." - "Invalid entries are logged and ignored" clause added — matches the
confluence_policy.py:177-180warning path and the existing Jira-section symmetry. - Endpoint enumeration corrected: "all page, space, search, and execute requests return 403" — covers
space/listandspace/pageswhich the previous wording elided.
Verified
- Fail-closed behavior on missing file / missing section / malformed YAML / non-list
spaces(confluence_policy.py:120-165). - Empty
spaces: []→ emptyfrozenset→ everyis_space_allowedreturnsFalse(confluence_policy.py:102-106). - Case-sensitivity claim matches Python set-membership semantics in
is_space_allowed. - CQL scope-enforcement pointer resolves to
docs/reference/confluence-wrapper.md(relative path is correct fromconfig/README.md). - Inline comment in
config/context-filters.yaml:26-42is consistent with the new README prose (no contradiction between the two operator-facing surfaces).
No new issues. Approving.
— Authored by egg
|
egg review completed. View run logs 3 previous review(s) hidden. |
…pdater] (#2149) * docs: document confluence.spaces in config/README.md Add confluence.spaces allowlist section to the context-filters.yaml documentation in config/README.md, mirroring the existing jira.projects section. The confluence gateway wrapper (PR #2141) added this configuration key but the README only covered the jira.projects allowlist. Authored-by: egg * docs: fix confluence.spaces regex and clarify wording Address reviewer feedback on PR #2149: - Correct the regex from `[A-Z][A-Z0-9_]*` to `[a-zA-Z][a-zA-Z0-9_]*` to match `_SPACE_KEY_RE` in gateway/confluence_policy.py:65 (the Confluence policy accepts mixed-case keys, unlike Jira). - Reword the case-sensitivity note with a concrete example so it does not compound with the (now corrected) regex. - Add the 'Invalid entries are logged and ignored' clause for symmetry with the jira.projects entry; behavior matches confluence_policy.py:178. - Broaden the empty-list 403 enumeration from 'page/search/execute' to 'page, space, search, and execute' so space/list is covered. Authored-by: egg --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
docs: document confluence.spaces in config/README.md
Update
config/README.mdto document theconfluence.spacesallowlist key in thecontext-filters.yamlsection. PR #2141 added the Confluence gateway wrapper and theconfluence.spacesconfiguration inconfig/context-filters.yaml, but the README only documented the paralleljira.projectssection.The new section mirrors the
jira.projectsdocumentation: format requirements, fail-closed behavior, empty-list semantics, hot-reload support, and a pointer to the CQL scope extractor rules in the Confluence Wrapper Reference.Triggered by: #2141
Authored-by: egg