Security Is Four Boundaries, and the Bug Lives in the Seam

There is no security module in this codebase.

Grep for one and you get a role vocabulary in Python, a CHECK constraint in SQL, a URL validator, a CDK policy statement, and a Next.js proxy that mints a per-request CSP nonce. Five files that never call each other, written in four different languages of enforcement.

Security here is a property that has to hold at four separate boundaries, and no single file owns it.

The request boundary, where a caller becomes a user holding verbs. The tenant boundary, where a query becomes scoped to an organization. The outbound boundary, where the server fetches something a user typed. The deploy boundary, where CI gets to touch production.

One pattern recurs at all four, and it is the part worth taking away. Make the unsafe state unrepresentable, not merely unreached.

Unreached is a property of today's call sites

Authorization in this app gates exactly five verbs: publish, approve, accept_memory_rule, manage_integrations, manage_team_billing. Drafting, editing, commenting, scheduling and reading are open to every member of an org. Nineteen route handlers mount a permission check; the other ninety-six do not.

That narrowness is what makes the next decision cheap. From backend/app_new/api/deps.py, with the docstring trimmed:

def require_permission(verb: str):

    async def _require(current_user: User = Depends(get_current_user)) -> User:
        if verb not in permissions_for_role(current_user.role):
            raise ForbiddenError(
                f"Your role '{current_user.role or DEFAULT_ROLE}' cannot "
                f"'{verb}'. Ask an admin to perform this action or change your role."
            )
        return current_user

    return _require

permissions_for_role used to return admin permissions for any role string it did not recognize. The justification was written into the code and sounds fine out loud: do not lock out legacy rows. It now returns the empty set, and logs authz.unknown_role so an operator sees drift instead of discovering it as an escalation. Known historical names keep their access through an explicit alias map, where approver resolves to manager.

The failure mode is a typo, a bad CSV import, or a future SSO path writing an unmapped string, and any of the three silently granting org admin. It was latent, because no route lets a user set their own role. It is exactly the landmine that detonates the first time roles arrive from an external identity provider.

Two existing tests had asserted the vulnerable behavior in so many words: missing role maps to admin, unmapped legacy role behaves as admin. The hole was encoded as intended behavior and passing CI.

Now the second half, which is the actual lesson. Failing closed means an unknown role is never acted on. It does nothing about an unknown role being written down. User.role was a String(20) with no enum and no constraint, so anything at all could land in that column and sit there looking legitimate.

backend/app_new/core/roles.py is the one place that answers which roles exist, and it renders its own SQL from that list: ROLE_CHECK_SQL = "role IN (" + ", ".join(f"'{r}'" for r in ALL_ROLES) + ")". Three assignable roles, four legacy ones kept valid so the constraint can never reject data the app already wrote.

That string is consumed by the models. From backend/app_new/models/user.py:

class User(Base):
    __tablename__ = "users"
    # Fail-closed authorization (api/deps.permissions_for_role) refuses unknown
    # roles at request time; this stops one being STORED at all. Legacy values
    # stay valid so the constraint can never reject data the app already wrote.
    __table_args__ = (CheckConstraint(ROLE_CHECK_SQL, name="ck_users_role"),)

backend/app_new/models/team.py carries the identical constraint on team_invites.role. The test for it writes role="okta:group:engineering" and asserts an IntegrityError, because that is the literal shape of what an SSO mapping produces when nobody normalizes it.

Unreached is a property of today's call sites. Unrepresentable is a property of the data. Only one of the two survives the next contributor.

Three safe-looking parts composed into one account takeover

On 2026-08-19, POST /api/v1/team/invite/accept could be used to take over any account by email address.

Three decisions produced it, and each one reads as reasonable on its own.

Registration is open, and every registrant becomes an admin of the new organization they just created. A self-serve signup needs somebody who can administer the new workspace.

Creating an invite returns the raw token in the response body. Nothing mails it anywhere, so the frontend has to be handed the join link to show the inviting admin.

And accepting an invite moves an existing account into the inviting org, rather than refusing, because a person invited to a second workspace already has an account and should not have to make another.

Each is defensible. Composed, they mean a stranger can register, invite your email address, receive the token in their own HTTP response, and post it to the accept endpoint. The handler verified the password only if body.password, so omitting the field entirely skipped authentication. Your account moved into their organization with the role they chose, and the endpoint handed them your tokens.

Verified exploitable: against the old code the attack returns 200.

The blast radius was zero. Production held two users, both the founder's, and zero invites had ever been created. This is a fix, not a disclosure.

The corrected existing-user path in backend/app_new/api/v1/team.py, comments trimmed:

    if user:
        from app_new.core.security import verify_password
        if not body.password or not verify_password(body.password, user.hashed_password):
            raise HTTPException(401, "Incorrect email or password")
        if not user.is_active:
            raise HTTPException(403, "This account is disabled. Contact your administrator.")
        user.organization_id = invite.organization_id
        user.role = invite.role

Three things changed. The password check is unconditional, because moving an account is an authentication event and not a convenience. A disabled account can no longer be silently re-activated through an invite link. And invites now expire after seven days, enforced at both places a token is looked up: the public preview endpoint GET /api/v1/team/invite/{token} and the accept endpoint. One site would have been a bug wearing a fix's clothing.

The regression test was written to fail against the vulnerable code first, then the fix was applied. It registers a victim, registers an attacker, invites the victim's email as admin, posts the token with no password, and asserts 401 with the message ACCOUNT TAKEOVER: accept with no password returned {status}. Its last assertion is that the victim can still log in to their own account.

The bug was not in any one of those three decisions. It was in the seam where they met, and no reviewer looking at one file would have seen it.

The tenant is the organization, and the user id is not a proxy for it

The invite handler leaves behind a very specific piece of state: a user whose organization_id changed while their user_id did not.

Seventeen of the twenty-six models carry an organization_id of their own, and the other nine are child rows reached through a parent that does. Twenty-six modules under backend/app_new/api/v1 filter on it. Two crew endpoints and all four notification queries filtered on user_id alone.

The failure mode is not theoretical, and the org move is what makes it reachable. After a move, the user's id still matches every conversation they created at their previous employer. Reading and appending to those conversations kept working. So did seeing that tenant's notifications.

Both crew lookups now go through one helper in backend/app_new/api/v1/crew.py, so there is a single predicate to audit:

    result = await db.execute(
        select(CrewConversation).where(
            CrewConversation.id == conversation_id,
            CrewConversation.user_id == user.id,
            CrewConversation.organization_id == user.organization_id,
        )
    )

That matches the _get pattern content/_shared.py and campaigns/_shared.py already used. The invite review eight days earlier had found the same shape one level down: a ContentVariant fetched by bare id, which let a variant belonging to another org be copied into your item. It is now scoped to its content item.

backend/tests/test_tenant_scoping.py does not test the endpoints in the abstract. It seeds exactly the post-move state, a conversation and a notification owned by the caller's user id but by an organization called "Previous Employer", and asserts 404 and absence. Removing the org predicate makes it fail, which is the only way to know a security test is load-bearing.

One detail that shapes all of this: the JWT carries an org claim, and nothing authorizes on it. get_current_user decodes the token, loads the User row, and every scoped query reads current_user.organization_id off that row. Tenancy is read fresh from the database on every request, so a token minted before an org move cannot carry stale tenancy into a query.

A hostname is not an address, and an address can change between checks

POST /api/v1/onboarding/scan-website lets any authenticated user hand the server a URL and have it fetched. Before hardening, the only check was a scheme prefix prepended to bare hostnames, and httpx followed redirects on its own.

That is a server-side request forgery primitive with an unusually good exfiltration channel. Registration is open, so the authentication requirement costs an attacker one signup. The caller then picks any address the server can reach, and the response comes back to them twice over: summarized by Claude in the API response, and persisted into org memory. On a cloud instance the highest-value target is 169.254.169.254, which returns IAM credentials wherever IMDSv2 is not enforced. The live cheap stack does enforce IMDSv2, so the exposure there is internal-network reach. The shipped simple stack does not.

backend/app_new/services/url_guard.py is the defense, and the ordering in it matters. Scheme allowlist first, which kills file://, gopher://, dict:// and ftp://. Then the address check, with trailing comments trimmed here:

def _is_public_address(ip: str) -> bool:
    addr = ipaddress.ip_address(ip)
    return not (
        addr.is_private
        or addr.is_loopback
        or addr.is_link_local
        or addr.is_multicast
        or addr.is_reserved
        or addr.is_unspecified
    )

is_link_local is the 169.254.0.0/16 test, which is where cloud metadata lives. The subtle part is the call site. validate_outbound_url resolves the hostname first and checks every resolved address, so a name pointing at 127.0.0.1 is caught, and a name resolving to both a public and a private address is refused outright rather than raced.

Then the hop that hostname validation alone would lose:

def guard_redirect(url: str) -> str:
    """Validate a redirect target. A public URL is free to 302 to metadata, so
    every hop is checked, not just the one the user typed."""
    return validate_outbound_url(url)

website_scanner.py sets follow_redirects=False and walks up to five hops itself, re-validating each one. With httpx following redirects, the library chases the hop before anything can inspect it. A body cap of 2 MB and a content-type allowlist of text/html, text/plain and application/xhtml+xml bound what comes back.

Refusals log the reason for the operator. The caller learns only that the scan failed, never which internal address was refused, because the error message is itself a probe result.

The test file covers what a pentester tries first: metadata over v4 and v6, loopback, RFC1918, 127.1, 0.0.0.0, non-http schemes, a hostname monkeypatched to resolve to loopback, mixed resolution, and a public URL that 302s into metadata. When the address check was disabled to check the tests were load-bearing, 12 of 24 failed.

The honest limit is written into the module's own docstring. Full DNS-rebinding defense requires pinning the validated IP into the connection, and this does not do that. Validating on resolved addresses beats validating on hostnames. It does not beat a resolver that answers differently on the second call.

The deploy role must not be able to write what it deploys past

Push to main and the pipeline deploys with no human approval gate. That puts the whole safety burden on what the CI role is allowed to touch.

Its ssm:PutParameter permission was originally scoped to the entire /commscrew/prod/* prefix. That prefix holds db_url, jwt_secret and anthropic_api_key alongside the four image-tag pointers a deploy actually writes. From infra/lib/commscrew-cheap-stack.ts:

    const tagParamArns = ['api_tag', 'web_tag', 'api_tag_prev', 'web_tag_prev'].map(
      (n) => `arn:aws:ssm:${this.region}:${this.account}:parameter${ssmPrefix}/${n}`,
    );
    deployRole.addToPolicy(new iam.PolicyStatement({
      actions: ['ssm:PutParameter'],
      resources: tagParamArns,
    }));

Read stays prefix-wide, harmless because the role has no kms:Decrypt. The ssm:SendCommand grant is split into two statements for the same reason: one statement covering both the shell-script document and instance/* would hand root on every instance in the account to anyone who can push to main. The instance statement is conditioned on the Name tag this stack sets on its own ASG.

The other half of an ungated pipeline is a gate that cannot be fooled. /health was a static dictionary returning 200 without consulting anything, so a container that booted with a dead database or a missing secret passed the swap gate, got promoted, and then served 500s while rollback never fired. /health/ready now runs SELECT 1 and, outside development, returns 503 if SECRET_KEY is still change-me-in-production or ANTHROPIC_API_KEY is unset. Both the on-box health check and the public smoke test point at the deep one.

Observability got the same treatment after post-deploy verification found /metrics publicly readable on the live API. From backend/app_new/main.py:

        cfg = get_settings()
        if cfg.METRICS_TOKEN:
            auth = request.headers.get("authorization", "")
            supplied = auth[7:] if auth.startswith("Bearer ") else ""
            if not (supplied and _secrets.compare_digest(supplied, cfg.METRICS_TOKEN)):
                return Response(status_code=401)
        elif cfg.ENVIRONMENT.lower() not in ("development", "dev", "test", "local"):
            return Response(status_code=403)

METRICS_TOKEN defaults to the empty string. In development that is open. In production it is a 403, not a 200, because an unconfigured guard has to be an outage and not a silence.

What these four boundaries do not cover

SECURITY.md lists thirteen enforced controls and four known limitations, and the first limitation eats a piece of the first boundary.

Access and refresh tokens live in localStorage, which means any script that executes on the page can read them. Logout clears local storage and does not revoke anything server-side, so a refresh token copied before logout stays valid for its full seven days. That is a demo posture, chosen so the SPA runs with no cookie or CORS setup, and the file names the four things production would need instead: an HttpOnly cookie, rotation with reuse detection, a server-side revocation list, and in-memory-only access tokens.

The nonce CSP in apps/web/src/proxy.ts exists because of that exposure specifically. Styles get 'unsafe-inline' and scripts never do, because script-src is the directive that stops XSS and an XSS here is a full account takeover rather than a defacement. A build guard fails pnpm build if any route is prerendered, since a nonce is minted during server rendering and a prerendered page never gets one.

Rate limits default to per-process counts, so N replicas enforce N times the configured limit unless RATE_LIMIT_SHARED_STORE is set. TRUSTED_PROXY_COUNT defaults to 0, which is correct when the app is exposed directly and wrong behind a proxy. The publishing OAuth flows are mocked and hold no real credentials.

The out-of-scope list at the top of that file exists so the documented posture is not re-reported as a finding. A reference architecture that quietly ships demo security teaches the demo security.

The pattern holds up better than any individual control here. At each of the four boundaries, the question to ask about a guard is whether the unsafe state is merely unreached today or actually cannot be represented. An unknown role gets no verbs, and the column will not store one. An expired invite is refused at both lookup sites, not the one you happened to read. Every conversation query carries both predicates through a single helper. Every redirect hop is re-resolved. The deploy role's ARN list has four entries in it.

The one-liner: security is four boundaries, and the bug is almost always in the seam between two of them.

Sibling post: the API layer, where the same fail-closed instinct shows up as rate limits keyed by what a request actually costs.

The views expressed here are my own and are not related to or reflective of my work or any organization I am affiliated with.