Conversation
The Linear project-to-repo mapping table is keyed on `linear_project_id` alone and carries no workspace attribute, so nothing ties a mapping to the tenant that owns it. The webhook processor looks the repository up from the body-supplied `projectId` with no way to check it against the workspace whose signature the delivery arrived with, which lets any onboarded workspace name another workspace's project and steer an agent task at that workspace's repository. This records the missing value so a later change can enforce it; no read path changes behaviour yet. `onboard-project` now resolves the owning workspace by asking Linear which onboarded workspace's own token can see the project, rather than accepting it as a flag. The resolution is what the enforcement path will check deliveries against, so a typo'd flag would durably write the cross-tenant mapping the check exists to prevent — and `organization.id` is read from the same authenticated response that resolved the project, not from the workspace we assumed we were asking. `--slug` narrows the search; `--workspace-id` records an owner without verifying it, for when the Linear API is unreachable. `backfill-project-workspaces` fills in mappings that predate the field, one pass per workspace rather than one lookup per row, and paginates the project listing (`list-projects` stops at 100, which for a backfill would silently leave the overflow unresolved). The update is conditional on the row still existing and still having no workspace id, so it neither resurrects a deleted mapping nor overwrites a concurrent `onboard-project`. A project id claimed by two workspaces is skipped and named rather than assigned to whichever answered first. `platform doctor` reports mappings with no owning workspace, capped at ten named ids, so an operator can tell whether the backfill is finished before enforcement is turned on.
… the event The processor resolved the target repository from the body-supplied `projectId` alone. The mapping table is keyed on that id with no workspace attribute, so a workspace holding its own valid signing secret could sign an event with its own `organizationId` while naming another workspace's project, and the processor would run an agent task on that workspace's repository. Nothing about that attack needs a shared or stack-wide secret, which is why signature-level fixes do not reach it. Now the mapping's `linear_workspace_id` is checked against the event's `organizationId` and a mismatch drops the delivery. Dropped rather than answered. The reply would be posted into the sender's own workspace, and "that project belongs to someone else" both confirms the project exists and tells a prober the attempt was seen. The log line carries both ids and is the diagnostic surface instead. A mapping with no owning workspace recorded still proceeds, and warns. Rejecting those on deploy would break every install that has not run `backfill-project-workspaces` yet; turning it into a rejection is a later step, gated per environment once `platform doctor` reports the backfill clean. Both rejection tests were confirmed to fail with the check disabled. The no-workspace-id case originally passed either way — an event with no `organizationId` never creates a task regardless, because it fails later at attribution — so it asserts the guard's own log line instead, which is the only thing that distinguishes a drop here from a drop there.
Verification could not tell a workspace's own signing secret from a copy of another workspace's, because the only available signal was value equality against the stack-wide secret — and that signal is wrong. A healthy single-workspace install also holds a secret equal to the stack-wide copy, since the first install stamps the same real secret into both slots. Rejecting on equality would 401 exactly the deployments that are safe. So ownership is now recorded by provenance instead: `webhook_secret_owned` on the registry row, written from where the value came from. `setup` sets it when the operator supplied the secret for this workspace or when a stored `lin_wh_` value differs from the stack-wide one. `update-webhook-secret` sets it too — that is the remediation path, and without it an operator could paste the correct secret and see no change in how the workspace is treated. Only a literal `true` is read back as proof. Absence means "not proven", which is what every row predating this field is, and is deliberately not the same as "proven not owned". `update-webhook-secret` gains `--stack-name` so it can reach the registry, and fails loudly if the secret write succeeds but the provenance write does not — silence there would leave the operator believing a fix landed while the state verification reads is unchanged. Two things found while building this, both worth knowing: `mirror-stackwide` — the branch that copies the first workspace's secret into an additional workspace — is already unreachable from `setup` on current main. #831 added an up-front prompt that throws when neither a supplied nor a legitimately stored secret exists, which is the only way into that branch. The refusal added here is therefore a safety net, not a live fix; it is extracted as `assertMirrorIsSafe` and unit-tested directly, because a guard reachable only through dead code is otherwise an untested guard. The remaining exposure is rows written by older releases and not re-run since. The provenance derivation cannot currently evaluate false, so mutating it to a literal `true` leaves every test passing. That is semantic equivalence under the current preconditions rather than a test gap — there is no reachable input that distinguishes them. It is kept as a derivation because it becomes load-bearing if that up-front prompt is ever relaxed, and the resulting failure would be silent.
…ace sent a delivery The processor routes from identifiers in the request body, so verification has to establish WHICH tenant sent a delivery — not merely that the sender knows a secret this stack accepts. Two secrets fail that test: the stack-wide one, which is bound to no workspace, and a per-workspace one that is a copy of another workspace's, which older releases could install. Both are now refused, and both only where they can actually cross a boundary: - `verifyLinearRequestForWorkspace` gains a `shared-secret` outcome. A matching signature on a row that does not record the secret as its own is rejected when another workspace is active. Fatal like `mismatch`, for the same reason — falling through would re-admit it via the stack-wide path. - The receiver skips the stack-wide fallback entirely on a multi-workspace stack. This is what supersedes sole-tenant binding rather than implementing it: with the fallback gone under multi-tenancy, that path survives only where there is one tenant, and there it cannot name the wrong one. - `verified_via_stack_wide` is forwarded to the processor, which replaces the body's `organizationId` with the sole active workspace and drops the delivery when there is no single answer. The field is overwritten on the payload rather than threaded as a parameter because the workspace id is read from about six places downstream; passing it alongside would leave every one of them a site where the claimed value could still be picked up. Single-workspace installs are untouched by design. Every check consults the active workspace count first, and with one tenant a tenant-less secret still identifies the only tenant there is. That also makes an absent provenance flag — the state of every row written before it existed — harmless on the installs where it is normal. `countActiveLinearWorkspaces` saturates at 2, since no caller needs the true total, and answers 1 when the registry cannot be read. That is deliberately the permissive direction: these checks harden a signature that has already verified, so a DynamoDB throttle must not become a stack-wide outage. No IAM change. `grantReadData` already covers `dynamodb:Scan` for both the receiver and the processor on the registry table. All three guards were confirmed to fail with the check disabled. One test needed fixing first: the multi-workspace fallback case passed because the harness shrank the workspace count along with the absent registry row, so the receiver had only ever seen one workspace.
Enforcing per-workspace secret ownership needs the provenance flag to already be set, and it is set only by `setup` and `update-webhook-secret`. On an existing multi-workspace stack no row has it, so turning enforcement on rejects every workspace at once — including workspaces that genuinely own their secret and are being broken for nothing. On the dev stack that is 3 of 3. `backfill-secret-provenance` closes the gap for the workspaces where ownership can actually be established. `mirror-stackwide` can copy nothing except the stack-wide value, so a stored secret that DIFFERS from it was never mirrored and is therefore owned. That conclusion cannot be wrong, which is what makes it safe to write. The converse is deliberately not used. A secret EQUAL to the stack-wide value is equally consistent with a healthy single-workspace install, whose first `setup` stamps the same real secret into both slots — so `inherited` withholds a verdict rather than asserting "not owned", and only `own` is recorded. This is the same asymmetry the earlier commit's doc comment warns about, now expressed as code: the direction that grants is sound, the direction that would reject is not. Reading the stack-wide secret is fatal on failure rather than treated as "not set". Continuing with undefined would classify every workspace as owning its secret, since nothing can equal a value that was never fetched — recording exactly the shared secrets this command exists to withhold. `platform doctor` reports the same population, gated on the active workspace count so it matches the enforcement path: on a single-workspace stack an unrecorded provenance is the normal state for every pre-existing row and carries no risk, so it passes with a note rather than warning. Both directions of the classifier were confirmed to fail when mutated — treating `equals stack-wide` as owned, and treating an empty stored secret as present.
… query The first live run of `backfill-project-workspaces` hit two workspaces whose Linear grants had expired. Both returned 401, so every mapping they own came back unresolved — and the report concluded those mappings "name projects no onboarded workspace can see" and told the operator to re-onboard or remove them. Four valid rows, and the suggested remedy was to delete them. An unresolved mapping means opposite things depending on whether every workspace was actually reachable, so the conclusion now branches on that. Skipped workspaces are tracked wherever the loop gives up on one — no token, an API error, or a response with no organization id — and when any exist the report leads with "Do NOT delete these yet", names them, and points at restoring their access first. The delete-them wording now appears only when every workspace was queried successfully. Verified by re-running against the dev stack, where the output now names demo-abca and maguireb as the reason rather than implicating their projects. The guard is asserted at source level: the branch sits inside a Commander action that would need the whole AWS and Linear surface stood up to reach, and the behavioural proof is the live run. The test pins that the unconditional wording cannot come back.
…ot be found `update-webhook-secret` writes two things: the signing secret, and the registry flag recording that the workspace owns it. Verification reads the flag, so without it the repair changes nothing observable — the workspace keeps 401ing. The registry write was wrapped in `if (registryTableName)`, so an operator who defaulted or mistyped `--stack-name` got the secret written, no flag, and a green "✅ Updated webhook signing secret". That is precisely the silent-miss the comment directly above the block claims to prevent, and it would send someone chasing a 401 whose cause was in the output they already saw. Now it refuses, names the stack it looked in, says the secret was preserved, and points at `--stack-name`.
…s in The fix itself is 257 lines of cdk/src. The CLI around it had grown to 926 — mostly tooling to migrate stacks into the new model. Reviewed against how many deployments actually need migrating, two pieces did not earn their weight. `backfill-project-workspaces` is gone, with its paginated project lister. A single-workspace install does not need it at all: with one tenant every mapped project belongs to that tenant, so a body-supplied projectId cannot steer a task across a boundary. Its real audience was "a stack with two or more workspaces AND pre-existing mappings", which is one deployment today. It was also the only part of the branch that needed live Linear tokens, and the only part that has already produced a defect — a report that told an operator to delete four valid mappings because two workspaces had expired grants. The doctor check that reported those mappings stays, because knowing about them is still worth an operator's attention. Its remedy is now "re-run onboard-project before you add a second workspace", and it says plainly that the state is harmless while one workspace is active. `assertMirrorIsSafe` is gone too. It guarded a branch that is unreachable from `setup`, and the value that branch would produce is no longer trusted on its own anyway — the receiver rejects a secret not recorded as the workspace's own once a second workspace is active. The comment there now describes what the code does rather than the refusal that used to follow it, and says what would need to change if the up-front prompt is ever relaxed. `backfill-secret-provenance` stays. This is the one place doing it by hand is actively dangerous rather than merely tedious: the safe rule holds in one direction only, and setting the flag on a workspace that holds the shared secret silently reopens the hole. It also runs with no Linear dependency at all, which is why it is the piece that already works on a stack with two dead grants. 935 tests green. The classifier's mutation was re-run after the removal, since it sat between two deleted blocks and had to be restored.
…tenant-binding # Conflicts: # cli/src/commands/linear.ts
The org-id fixtures use hex letters in the final group because a UUID ending in twelve consecutive digits matches gitleaks' `aws-account-id` rule, which failed CI's range-scoped secret scan on values that hold no secret. The values themselves were corrected in place across this branch's history — fixing them in a follow-up commit left them in the commit that introduced the file, and the scan is range-scoped over every commit in the PR. Recorded as a comment so the constraint survives the next person editing these constants. Fixed in the fixture rather than allowlisted in `.gitleaks.toml`: widening a shared security rule to accommodate a test constant is the wrong direction.
isadeks
force-pushed
the
fix/linear-webhook-tenant-binding
branch
from
September 22, 2026 20:34
58c306b to
28d7a03
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
A Linear webhook delivery is routed using identifiers that arrive in the request body: which workspace sent it, which project it belongs to, and which user to attribute the resulting task to. Verification established that the sender knew a signing secret this stack accepts, but nothing tied those routing values to the workspace the signature actually belonged to. This binds them.
Three things now hold:
Single-workspace installs are unaffected
Every new check consults the number of active workspaces first. With one workspace a secret that is not bound to a particular workspace still identifies the only workspace there is, so each check is inert and no existing behaviour changes. This is the case the tests pin explicitly, because it is the configuration almost every deployment is in.
Multi-workspace installs need one command first
Run this before deploying, and read the dry run:
It records ownership for every workspace whose stored secret differs from the stack-wide value, which proves that secret was never copied from it. Workspaces it cannot establish ownership for are named, with the command to fix each.
bgagent platform doctorreports the same state, and stays quiet on a single-workspace stack.Project mappings created before this change record no owning workspace. Those are reported by
platform doctorand are harmless while one workspace is active; re-runonboard-projectfor each before adding a second.Verification
Each new guard was disabled in turn and the test written for it was confirmed to fail, then restored — re-run after merging
main, sincemainhad modified four of the six files this touches. Two of those runs found weak tests rather than weak code: one passed for an unrelated reason and now asserts the guard's own log line.The provenance backfill and the project-owner resolution were both run against a real three-workspace deployment. That run corrected a report that would have told an operator to delete four valid mappings when two workspaces simply had expired credentials.
Scope deliberately not taken
An earlier revision carried a second backfill command for project mappings. It needed live provider credentials, served installs that are already multi-tenant with pre-existing mappings, and was the only part of the change that had produced a defect of its own — so it was removed in favour of re-running
onboard-project. The remaining backfill stays because the rule it encodes is sound in one direction only, and getting it backwards by hand would silently undo the fix.Before this leaves draft
🤖 Generated with Claude Code