Skip to content

feat(integrations): check that the GitLab token can read the repository - #107637

Open
ablaszkiewicz wants to merge 2 commits into
feat/et-repo-files-git-listerfrom
feat/gitlab-token-read-check
Open

ablaszkiewicz wants to merge 2 commits into
feat/et-repo-files-git-listerfrom
feat/gitlab-token-read-check

Conversation

@ablaszkiewicz

@ablaszkiewicz ablaszkiewicz commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Merge order

  1. feat(error-tracking): list repository files with git #107631 feat(error-tracking): list repository files with git
  2. feat(error-tracking): store the file list of each release #107632 feat(error-tracking): store the file list of each release
  3. 👉 feat(integrations): check that the GitLab token can read the repository #107637 feat(integrations): check that the GitLab token can read the repository
  4. feat(cymbal): add the path resolution service #107633 feat(cymbal): add the path resolution service
  5. feat(cymbal): add repo paths to frames #107634 feat(cymbal): add repo paths to frames
  6. feat(error-tracking): link frames straight to their repo path #107635 feat(error-tracking): link frames straight to their repo path
  7. feat(error-tracking): scroll a frame's file at the release commit #107636 feat(error-tracking): scroll a frame's file at the release commit

Problem

  • A GitLab integration set up with a token that lacks read_repository passes setup, then fails later without a message when error tracking lists the repository files with git (feat(error-tracking): store the file list of each release #107632).
  • GitLab setup accepts any token that can call the project API, so the person who set it up never learns the token is too narrow.

Changes

  • GitLab setup now rejects, with a message in the setup modal:
    • a project the token cannot read: "Couldn't read the GitLab project. Check the project ID and that the token belongs to this project."
    • a token that git refuses: "This token can't read the repository. Create a project access token with the read_repository scope and try again."
  • The check runs git ls-remote with the lister from feat(error-tracking): list repository files with git #107631, with a 15 s timeout. The host is the one the project API call already validated, and git redirects are off.
  • When the check cannot run (network failure, timeout, non-HTTPS host), setup continues, so an outage does not block setup.
  • The setup modal help names the role and the scopes to use:
- Learn how to create a project access token
+ Use the Reporter role and the api and read_repository scopes.
+ PostHog reads the repository to link stack frames to files. Create a project access token
  • Mechanical: an IntegrationError from GitLab setup maps to a 400 validation error, so the modal shows the message.

Warning

This applies to every new GitLab integration, not only teams with the error tracking flag. A token without read_repository that setup accepted before is now rejected. Existing integrations are not checked again.

How did you test this code?

  • TestGitLabIntegration.test_create_checks_the_token_can_read_the_repository catches a token git refuses that still creates an integration, a failed check that blocks setup, and a project response without a path that creates a broken integration.
  • The GitLab auth header format was checked against gitlab.com with a project access token.
  • Not checked: the modal copy in a rendered UI. The change is text only.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None. No doc under docs/ covers GitLab integration setup.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5, 1M context)

  • Session: https://claude.ai/code/session_01DpTwx89x8Wu9Nu9B82mPEj
  • Skills invoked: /writing-user-facing-copy, /writing-tests, /writing-code-comments, /writing-pr-descriptions.
  • CodeRabbit CLI: skipped. The CLI was signed out, and the person chose to skip setup.
  • The check lives in the error tracking facade, because the git lister belongs to that product. The integration model imports it inside the function to avoid a circular import.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DpTwx89x8Wu9Nu9B82mPEj

@ablaszkiewicz ablaszkiewicz self-assigned this Sep 28, 2026
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Complexity (TypeScript) — clean

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Comment density — 4% of added code lines are comments (4 of 111)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
posthog/models/integration/gitlab.py 4 17

This check does not block merging. It updates on every push and clears when the share drops.

⚠️ Bundle size — 🔺 +181 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.94 MiB · 🔺 +181 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.57 MiB · 22 files no change █████████░ 85.5% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.56 MiB · 629 files no change █████████░ 88.4% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.37 MiB · 2,330 files no change █████████░ 88.4% of 8.34 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
854 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
267.6 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.4 KiB src/lib/api.ts
88.4 KiB src/products.tsx
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
40.1 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
28.4 KiB src/scenes/scenes.ts
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
271.7 KiB src/taxonomy/core-filter-definitions-by-group.json
267.6 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.4 KiB src/lib/api.ts
98.5 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
88.4 KiB src/products.tsx

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.38 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.38 MiB · 19 files no change ████░░░░░░ 41.5% of 5.72 MiB
Deferred (lazy) 2.10 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
800.0 KiB dist/toolbar/toolbar-app-6DYXIWRU.css
651.4 KiB dist/toolbar/chunk-chunk-TVTQKACM.js
483.6 KiB dist/toolbar/chunk-chunk-6JFSEK3E.js
138.3 KiB dist/toolbar/chunk-chunk-CP4QT72J.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-OHYAIWFE.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-577JVGVT.js
21.0 KiB dist/toolbar/chunk-chunk-GNB7IYR7.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +1.7 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 946.14 MiB · 🔺 +1.7 KiB (+0.0%)

✅ Playwright — all passed

All tests passed.

View test results →

@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[High risk] Adds token validation to GitLab integration setup.

No findings were recorded in the synthesis handoff.

Reviews (1) · Last reviewed commit: "feat(integrations): check that the GitLa..."

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 8598ad36-c24d-4fab-a9be-598a3d5388a8

📥 Commits

Reviewing files that changed from the base of the PR and between 96e385d and beaa38b.

📒 Files selected for processing (3)
  • posthog/models/integration/gitlab.py
  • products/error_tracking/backend/facade/api.py
  • products/error_tracking/backend/test/test_facade_api.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

GitLab integration creation now validates the project path and checks repository readability through a new error-tracking facade probe. The API converts integration errors to validation errors. Tests cover readable, unreadable, unavailable-check, and project-lookup-error cases. The setup modal specifies the Reporter role and required token scopes.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to beaa3

GitLab integration setup now checks repository readability. A slow GitLab response can hold a database connection for up to 15 seconds during setup. This is bounded and affects only GitLab setup, so it is acceptable to merge with awareness or a follow-up.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to beaa3

The access check improves setup feedback, but slow repository probes can now occupy an integration-creation request while its database transaction is open. The check has a timeout and restricted callers; the impact of repeated slow requests remains uncertain.

Retained concerns

  • Medium · security · inferred: Integration creation now holds a request and database transaction across an additional authenticated Git subprocess. Repeated slow probes could consume shared request and database capacity; the effective concurrency limits are unproven.
Security review details

Security Blast Radius

  • inferred — An authenticated project member can initiate the new outbound Git probe through integration creation. Slow concurrent requests could affect capacity shared beyond that member's project, but effective throttling and worker capacity were not established.

Security Findings and Attack Paths

  • inferred — The introduced availability path is repeated, slow integration creation rather than an established credential-leak path. Each Git command is deadline-bound and its process group is cleaned up, which limits—but does not establish an aggregate limit on—concurrent requests.

Trust Boundaries and Controls

  • observed — The project API response supplies the repository path, while the submitted token supplies Git authority. The resulting remote is checked again for protocol, destination, proxy, and redirect behavior before Git connects; explicit Git authentication failure blocks setup.

Resilience and Maintainability Implications

  • observed — Timeout and other probe failures are logged by error type and treated as indeterminate, preserving setup availability. This is not a guarantee that a saved integration can read repository files; the prior setup also had no repository-read gate.

Hardening Proposals

  • proposed — Consider bounding concurrent repository probes or moving the network check outside the integration-creation transaction while retaining a final authorization and persistence check.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description follows the required template and explains the problem, user-visible changes, testing, release status, documentation status, and agent context. It notes that the frontend copy was not …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
posthog/api/test/test_integration.py (1)

1112-1112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Annotate both new test signatures.

The Python guidance requires annotations on every signature. Add concrete annotations for db, all parametrized inputs, and None return types to setup_integration and test_create_checks_the_token_can_read_the_repository. The test guidance provides no exception to this rule.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 586b1f7c-a38b-4ca5-bb9c-75d9096f7d58

📥 Commits

Reviewing files that changed from the base of the PR and between 4e4f5a3 and 5507115.

📒 Files selected for processing (5)
  • frontend/src/scenes/integrations/gitlab/GitLabSetupModal.tsx
  • posthog/api/integration.py
  • posthog/api/test/test_integration.py
  • posthog/models/integration/gitlab.py
  • products/error_tracking/backend/facade/api.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 0 remain after this review.

Comment thread posthog/models/integration/gitlab.py Outdated
@trunk-io

trunk-io Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
the activity log logic humanizing insights can handle change of insight query as a query wrapped in an InsightVizNode The test failed because the code could not find the 'models.cohortsModel' path in the store. Logs ↗︎
the activity log logic humanizing insights can handle change of a SQL insight query The test failed because it could not find the specified path 'scenes.PreflightCheck.preflightLogic' in the store. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

Error tracking lists repository files with git, which needs a project access
token with the read_repository scope. GitLab setup now rejects a token that
git refuses, and a project the token cannot read, with a clear message. The
setup modal names the role and scopes to use.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The read check returns None, and logs the skip, when git raises an
  OSError (no git binary, no temp directory). Before, the error reached
  the request and GitLab setup failed with a 500.
- The rejection message names the Reporter role as well as the
  read_repository scope, because a Guest-role token also cannot read.
- The tri-state result has a name, and the stored config reuses the
  checked path_with_namespace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8GihphVSmUSCXNkx4GTpg
@posthog

posthog Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time. Enjoy the moment:

The dancing man in the red room from Twin Peaks

Resolved comments: 1 declined

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit beaa38b · box box-e7ce99dfaae8 · ready in 631s (push → usable) · build log · rebuilds on every push, torn down on close

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team and hpouillot September 29, 2026 20:55

This branch was successfully deployed

1 active deployment
preview-pr-107637 — beaa38b1 Deployed Sep 29, 2026 by github-actions[bot]
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