insights/.agents/security.md

Security guidelines for agents

Principle of Least Privilege

Default to the smallest permission, narrowest field set, and shortest scope that still works. If a change needs more access than what's already in place, stop and reconsider the design before widening it.

• Default deny. Start from no access and add what's required, rather than opening things up and trimming back.
• Scope every queryset. Filter by team_id (and organization_id where appropriate) so a request can only ever touch its own data.
• Reuse existing permission and access-control classes — don't write a looser variant to fit a new caller. If the existing class doesn't fit, that's a design conversation, not a quick relax.
• Separate read and write checks. "Can see" and "can modify" are different questions; don't let one imply the other.
• Lock down ownership fields. Mark team, created_by, organization, and similar fields as read_only on serializers. Set them server-side from request context, never from client input.
• Expose the minimum on serializers. Don't include tokens, secrets, internal IDs, or fields a caller doesn't need just because they exist on the model.
• Gate admin and internal paths explicitly. Use is_staff, organization roles, or feature flags — never rely on URL obscurity or "no one will hit this".
• Background work runs with caller scope. Celery tasks and Temporal activities should preserve the requesting user/team, not silently escalate to superuser.
• Tokens and credentials: narrowest scope, shortest lifetime. Don't widen an existing token's scope to fit a new use case — mint a new one. Never log secrets, never commit them.
• Remove access in the same change as the feature. When a role, endpoint, or capability goes away, delete its permissions and grants then and there. Dormant grants accumulate and become tomorrow's incident.

Secrets & key management

SECRET_KEY already backs Django session/CSRF signing and is the legacy default for several other keys. Treat it as load-bearing and frozen — every new signing/encryption need gets its own key, rotatable and per-environment from the start.

• Don't extend SECRET_KEY to new purposes. Do not add code that reads settings.SECRET_KEY (or derives keys from it) for a new job. Mint a dedicated setting in insights/settings/ read from its own env var — e.g. <PURPOSE>_SIGNING_KEY / <PURPOSE>_SECRET_KEYS. A genuinely new purpose must NOT fall back to SECRET_KEY; fail closed when unprovisioned instead. Canonical example: RECORDING_API_JWT_SECRET (insights/settings/session_replay_v2.py) — no SECRET_KEY default, empty in prod so only designated minters can sign. (Older keys like JWT_SIGNING_KEY, TEMPORAL_SECRET_KEY, FLAGS_SECRET_KEYS default to SECRET_KEY for self-hosted back-compat; that is migration debt, not the pattern to copy for new keys.)
• Support rotation from day one. Read the key as a list, newest first: the first element signs/encrypts, all elements are tried for verify/decrypt. Prefer a single comma-separated env var (KEY="<new>,<old>") parsed with get_list() — the Fernet new,old convention. Examples: RECORDING_API_JWT_SECRET (recording_api_jwt.py), FLAGS_SECRET_KEYS (encrypted_flag_payloads.py). For encryption use MultiFernet (encrypt with first, decrypt with any) — see insights/helpers/encrypted_fields.py. Encrypting data at rest? ship a re-encryption path that re-wraps onto the primary key, like insights/management/commands/reencrypt_fields.py. The primary + _FALLBACKS pair (JWT_SIGNING_KEY / JWT_SIGNING_KEY_FALLBACKS) is the older two-var variant; single-var comma-separated is preferred for new keys.
• Unique value per environment — including prod US and prod EU. Never share a key value across environments; provision each (dev, prod-US, prod-EU) independently so one region can rotate without breaking another and a leak in one doesn't compromise the rest. Enforced at provisioning, not in code — so never hardcode a shared prod default. Fail closed when a prod key is unset (mirror the SECRET_KEY startup guard in insights/settings/access.py and RECORDING_API_JWT_SECRET's empty-in-prod default).
• Don't extend INTERNAL_API_SECRET to new service-to-service calls. It's a single fleet-wide shared secret already trusted by many services across Django, Node, and Rust — one leak reaches all of them — so it must not grow new callers or protected endpoints (it's being actively retired edge-by-edge; adding one moves it backwards). For a new internal call, in order of preference: (1) mint a scoped JWT (strongly preferred) — a dedicated per-audience signing key (never INTERNAL_API_SECRET/SECRET_KEY/JWT_SIGNING_KEY) with claims pinning the token to its team and operation, verified per-route; canonical example RECORDING_API_JWT_SECRET (mint in insights/session_recordings/recordings/recording_api_jwt.py, verify in nodejs/src/session-replay/recording-api/auth.ts). (2) If a JWT genuinely doesn't fit, a dedicated static secret scoped to that one caller→callee pair (<PURPOSE>_API_SECRET, its own env var, rotatable, empty-in-prod-fail-closed) — never reuse INTERNAL_API_SECRET. The goal is blast radius: no single "master" credential that authenticates to a dozen services — a scoped JWT limits a leak to one team + operation; a dedicated secret limits it to one hop.

SQL Security

• Never use f-strings with user-controlled values in SQL queries - this creates SQL injection vulnerabilities

• Use parameterized queries for all VALUES: cursor.execute("SELECT * FROM t WHERE id = %s", [id])

• Table/column names from Django ORM metadata (model._meta.db_table) are trusted sources

• For Datastore identifiers, use escape_datastore_identifier() from insights/insightsql/escape_sql.py

• When raw SQL is necessary with dynamic table/column names:

  # Build query string separately from execution, document why identifiers are safe
  table = model._meta.db_table  # Trusted: from Django ORM metadata
  query = f"SELECT COUNT(*) FROM {table} WHERE team_id = %s"
  cursor.execute(query, [team_id])  # Values always parameterized
  

InsightsQL Security

InsightsQL queries use parse_expr(), parse_select(), and parse_order_expr(). Two patterns exist:

Vulnerable pattern - User data interpolated INTO a InsightsQL template:

# User data embedded in f-string - can escape context!
parse_expr(f"field = '{self.query.value}'")  # VULNERABLE

Safe patterns:

# User provides ENTIRE expression - no context to escape
parse_expr(self.query.expression)  # SAFE - InsightsQL parser validates syntax

# User data wrapped in ast.Constant placeholder
parse_expr("{x}", placeholders={"x": ast.Constant(value=self.query.field)})  # SAFE

Why direct pass-through is safe: When users provide the entire InsightsQL expression (not data embedded in a template), there's no string context to escape from. The InsightsQL parser validates syntax and rejects malformed input.

Sanitizers (for use in placeholders):

• ast.Constant(value=...) - wraps values safely
• ast.Tuple(exprs=...) - for lists of values
• ast.Field(chain=[...]) - wraps identifiers (table names, column names) safely

ORM Security

Django's __ notation in ORM lookups allows FK traversal. If a user-controlled value is interpolated into a .filter(), .exclude(), or Q() dict key, an attacker can traverse relationships to exfiltrate sensitive fields (e.g. user__password, team__api_token).

Vulnerable pattern - Variable interpolated into filter key:

# key is user-controlled — attacker can pass "user__password"
queryset.filter(**{f"{key}__icontains": value})  # VULNERABLE

Safe patterns:

# Validate against an allowlist before use
ALLOWED_FIELDS = {"name", "email", "created_at"}
if key not in ALLOWED_FIELDS:
    raise ValueError(f"Invalid filter field: {key}")
queryset.filter(**{f"{key}__icontains": value})  # SAFE

# Hardcoded keys are always safe
queryset.filter(**{"name__icontains": value})  # SAFE
queryset.filter(name__icontains=value)  # SAFE

JSONField note: If the first path segment is a JSONField (e.g. detail__), Django routes all subsequent __ as JSON key lookups rather than FK traversals, which mitigates the risk. An allowlist is still recommended as defense in depth.

Deserialization (pickle)

Do not add new pickle usage. Unpickling bytes that any other process could influence is arbitrary code execution — the pickle stream can name any importable callable and pass it arguments. This applies to the equivalents too: cPickle, cloudpickle, dill, marshal, shelve, joblib, pandas.read_pickle, and numpy.load(..., allow_pickle=True). CI already fails on new pickle: the avoid-pickle rule from the p/security-audit Semgrep pack runs with --error (see .github/workflows/ci-security.yaml), so introducing one is a hard stop, not a warning. Pickle can also appear without the word pickle: Django's default cache backend (cache.set / cache.get) pickles values transparently, so treat the cache as a pickle boundary and only store JSON-serializable values in it.

Use JSON instead. For plain data, use json. For typed objects, use a schema-validated (de)serializer that constructs only declared types — Pydantic model.model_dump_json() / Model.model_validate_json(...) is the house default. Anything that crosses a process, cache, queue, or the wire must be JSON-shaped, not pickle-shaped.

If pickle is genuinely unavoidable — almost always an existing cross-language or cross-version format contract you can't change unilaterally (e.g. Django's cache backend and the Rust services both wrap JSON in pickle) — it must satisfy all of:

1. Internal, self-generated data only. Never unpickle anything a user, tenant, or untrusted service could write. "Internal Redis" is not a trust boundary — anyone with write access to the datastore controls the bytes.
2. A restricted pickle.Unpickler. Override find_class to a strict allowlist of the exact classes you expect, or — when the payload is JSON-shaped (str/dict/list/number/bool/None) — a deny-all find_class that raises for everything. JSON-shaped values deserialize through dedicated opcodes that never call find_class, so denying it blocks every code-execution path while still loading your data.
3. A justified # nosemgrep on the line immediately before the call (per the repo nosemgrep convention), stating why it's safe.

Reference implementations:

# Deny-all: the payload is only ever a JSON string / dict, so no class is ever legitimate.
# insights/storage/auth_token_cache_verifier.py -> _JsonOnlyUnpickler
class _JsonOnlyUnpickler(pickle.Unpickler):
    def find_class(self, module: str, name: str) -> Any:
        raise pickle.UnpicklingError(f"refusing to unpickle disallowed global {module}.{name}")

# Allowlist: reconstruct only our own catalog classes.
# insights/insightsql/database/database.py -> _CatalogUnpickler
class _CatalogUnpickler(pickle.Unpickler):
    def find_class(self, module: str, name: str) -> Any:
        if module.startswith(_ALLOWED_PREFIXES) or module in _ALLOWED_MODULES:
            return super().find_class(module, name)
        raise pickle.UnpicklingError(f"refusing to unpickle disallowed global {module}.{name}")

Semgrep Rules

Run semgrep --config .semgrep/rules/security/ . to check for injection issues.

Three rules:

1. insightsql-injection-taint - Flags user data (self.query.*, self.$obj.$field, etc.) interpolated into f-strings passed to parse functions (HIGH confidence)
2. insightsql-fstring-audit - Flags all f-strings in parse functions for manual review (LOW confidence)
3. orm-field-injection - Flags variables interpolated into dict keys passed to .filter(), .exclude(), or Q() (MEDIUM confidence)

When semgrep flags your code:

• If user data is interpolated into an f-string InsightsQL template → wrap with ast.Constant() or ast.Field(chain=[...]) in placeholders
• If a variable is used in an ORM filter key → validate against an allowlist of permitted field names
• If the code is safe (loop index, enum, dict lookup, JSONField prefix) → add # nosemgrep: <rule-id> with explanation

Running tests:

# Local install
semgrep --test .semgrep/rules/security/

# Or via Docker
docker run --rm -v "${PWD}:/src" semgrep/semgrep semgrep --test /src/.semgrep/rules/security/