Skip to content

Correct access control docs and widen the docs review rule - #2346

Merged
kmcginnes merged 4 commits into
mainfrom
access-control-docs-review-fixes
Oct 1, 2026
Merged

kmcginnes merged 4 commits into
mainfrom
access-control-docs-review-fixes

Conversation

@kmcginnes

@kmcginnes kmcginnes commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #2343, from a review that ran after it merged.

Access control docs

  • Narrow "any request that reaches the proxy server is served". The allowlist and header validation both reject requests, so the sentence overstated it. It now says what it meant: the proxy server never checks who the caller is.
  • Note that the /explorer segment is matched case-sensitively. The proxy matches API paths case-insensitively, but the UI finds /explorer in its own URL with a case-sensitive match, so a layer that changes that segment's casing breaks the UI. The path matching paragraph implied casing never mattered.
  • Say what the missing header error contains. The reference said nothing in the response indicates a stripped graph-db-connection-url. The 400 body names the missing header, which is the clue a layer operator needs.
  • Name SageMaker's access control layer under the shared block. The block tells readers to put an access control layer in front, and on SageMaker the notebook's Jupyter proxy already is one. A sentence after the block points to the guide's Security model section, so the block itself stays identical across the four guides.
  • Reword the troubleshooting caveat. It said "the paths listed here" after Document the deployer's responsibility for access control #2343 replaced that list with a link to the table.
  • Use [!IMPORTANT] for the getting started notice, matching the same prohibition in the four deployment guides. It was a [!NOTE].
  • Fold the EC2 guide's network disclaimer into the intro. The guide opened with three alerts in a row. The two [!IMPORTANT] blocks stay, since both appear in every deployment guide.

Review and doc conventions

  • REVIEW.md: widen the docs bullet. It only covered docs contradicting code, and still named CONTEXT.md, which is now GLOSSARY.md. It now covers docs contradicting each other, names what counts as Important, and drops the file list. It was the only bullet that applied to a docs-only change.
  • REVIEW.md: scope "Writing comments" to the reviewer's own comments. A reviewer applied those rules to the docs under review.
  • docs/agents/documentation.md: one marker per claim. Alert markers had no consistency rule, which caused most of the nits in that review.
  • docs/agents/documentation.md: record which docs mirror code. The reference section copies the proxy server's routes, headers, and limits by hand, and nothing told an agent to update it alongside the code.

How to read

  1. docs/references/security.md — the two content corrections
  2. REVIEW.md — the review rule changes
  3. docs/agents/documentation.md — the marker rule and the docs that mirror code

The rest are one-line edits.

Validation

  • pnpm checks passes.
  • Docs and review conventions only, no code changed.
  • The 400 body was checked against RequestValidationError in packages/graph-explorer-proxy-server/src/errors.ts, which carries the prettified Zod error naming the field.
  • The case-sensitive /explorer match was checked against resolveApiRoot in packages/graph-explorer/src/connector/utils/apiUrl.ts.

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

@kmcginnes kmcginnes left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approve

@kmcginnes
kmcginnes marked this pull request as ready for review October 1, 2026 20:40
@kmcginnes
kmcginnes merged commit 3f2a4f2 into main Oct 1, 2026
1 check passed
@kmcginnes
kmcginnes deleted the access-control-docs-review-fixes branch October 1, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant