docs: document implemented architecture and multi-tenant security model #64
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| # Multi-tenant security model | ||
|
|
||
| Where the tenant and environment boundaries actually sit, what enforces them, | ||
| and what PostKit does not enforce yet. | ||
|
|
||
| Implemented in | ||
| [`apps/api/src/tenant/api-key-tenant-resolver.ts`](../../apps/api/src/tenant/api-key-tenant-resolver.ts), | ||
| [`apps/api/src/templates/blob-template-store.ts`](../../apps/api/src/templates/blob-template-store.ts), | ||
| [`apps/api/src/functions/send.ts`](../../apps/api/src/functions/send.ts), and | ||
| [`packages/post-kit-publisher/src/path-safety.ts`](../../packages/post-kit-publisher/src/path-safety.ts). | ||
|
|
||
| ## Tenant identity comes from the credential | ||
|
|
||
| A caller sends: | ||
|
|
||
| ```text | ||
| Authorization: Bearer <token> | ||
| ``` | ||
|
|
||
| `ApiKeyTenantResolver` looks the token up in a `TenantKeyMap` and returns a | ||
| `TenantContext` of `{ tenantId, environment }`. The map is injected at | ||
| construction — in production it is parsed from the `TENANT_KEY_MAP` | ||
| environment variable as JSON (an unparseable value yields an empty map, so | ||
| every request then fails closed with `403 UNAUTHORIZED`). | ||
|
|
||
| Shape, with placeholder values only: | ||
|
|
||
| ```json | ||
| { | ||
| "<token-for-acme-production>": { "tenantId": "acme", "environment": "production" }, | ||
| "<token-for-acme-development>": { "tenantId": "acme", "environment": "development" } | ||
| } | ||
| ``` | ||
|
|
||
| Consequences that follow directly from this design: | ||
|
|
||
| - **One credential maps to exactly one tenant *and* one environment.** There | ||
| is no way for a caller to select a tenant or an environment per request. A | ||
| tenant that needs `development` and `production` access needs two tokens. | ||
| - **`tenantId` is never accepted from the request body.** `SendRequest` has no | ||
| tenant field, and the handler only ever uses the resolver's output. | ||
| - Resolver failures are distinguished: a missing `Authorization` header, a | ||
| non-Bearer scheme, or an empty token give `401 UNAUTHENTICATED`; a | ||
| syntactically fine token that is not in the map gives `403 UNAUTHORIZED`. | ||
| - The Bearer scheme is matched case-insensitively (`bearer` is accepted) with | ||
| one or more spaces before the credential. | ||
| - Token lookup uses `Object.prototype.hasOwnProperty`, so prototype-chain | ||
| names such as `toString` or `__proto__` cannot be used as valid tokens. | ||
|
Comment on lines
+47
to
+48
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/sh
set -eu
node <<'NODE'
const parsed = JSON.parse('{"__proto__":{"tenantId":"acme","environment":"production"}}');
if (!Object.prototype.hasOwnProperty.call(parsed, '__proto__')) {
throw new Error('Expected an own __proto__ property');
}
console.log('confirmed');
NODERepository: singleton-sd/post-kit Length of output: 169 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/singleton-sd-post-kit-c40aa8a1 -type f -name '*.md' -print
for f in /tmp/coderabbit-repo-knowledge/singleton-sd-post-kit-c40aa8a1/*/*.md; do
[ -f "$f" ] || continue
case "$f" in
*learnings*|*architecture*) continue ;;
esac
printf '\n--- %s ---\n' "$f"
cat "$f"
done
printf '%s\n' '--- target documentation ---'
cat -n docs/architecture/multi-tenant-security.md
printf '%s\n' '--- parseTenantKeyMap definitions and callers ---'
rg -n -C 8 'parseTenantKeyMap|hasOwnProperty|JSON\.parse' --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: singleton-sd/post-kit Length of output: 50379 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- candidate API files ---'
fd -t f . apps/api/src | sort
printf '%s\n' '--- targeted resolver and send-handler references ---'
rg -n -C 12 'TenantKeyMap|ApiKeyTenantResolver|parseTenantKeyMap|hasOwnProperty' apps/api/srcRepository: singleton-sd/post-kit Length of output: 41774 Remove
🤖 Prompt for AI Agents |
||
| - The token value is never included in an error message or a log entry. Logs | ||
| carry only the declared `LogEntry` fields — `correlationId`, `tenantId`, | ||
| `environment`, `templateKey`, `outcome`, `durationMs`, `providerMessageId`, | ||
| and `errorCode`. | ||
|
|
||
| The Azure Functions binding uses `authLevel: 'anonymous'`. That is | ||
| deliberate: PostKit performs its own authentication, and no Functions host key | ||
| is involved in tenant identity. | ||
|
|
||
| ## Environment separation is a storage-path boundary | ||
|
|
||
| `TenantEnvironment` is `'development' | 'staging' | 'production'`, and the | ||
| environment from the credential is interpolated straight into the blob path: | ||
|
|
||
| ```text | ||
| tenants/{tenantId}/{environment}/templates/{templateKey}/… | ||
| ``` | ||
|
|
||
| So a development credential physically cannot read a production template | ||
| artifact — it resolves a different blob prefix, and a missing blob returns | ||
| `404 TEMPLATE_NOT_FOUND`. This is enforcement, not just a naming convention, | ||
| because the path is derived from server-side state the caller cannot | ||
| influence. | ||
|
|
||
| On the publish side, `post-kit-publisher` asserts the environment is one of | ||
| the three known values before building any path, so a typo cannot create a | ||
| fourth pseudo-environment directory. | ||
|
|
||
| The read side does not have that assertion. `parseTenantKeyMap()` in the send | ||
| handler casts the parsed `TENANT_KEY_MAP` JSON to `TenantKeyMap` without | ||
| validating it, and `BlobTemplateStore.load()` interpolates `tenantId` and | ||
| `environment` into the path without re-checking them (only `templateKey` is | ||
| re-validated). A malformed map entry — a `tenantId` containing `/` or `..`, or | ||
| an `environment` outside the three known values — would therefore produce an | ||
| unintended blob prefix. `TENANT_KEY_MAP` is trusted operator configuration, not | ||
| caller input, so this is a configuration-integrity concern rather than a | ||
| request-level bypass; see **What is not enforced yet**. | ||
|
|
||
| Note the scope of the boundary: it isolates **template content**. Provider | ||
| credentials, the from-address, and the storage account are process-level | ||
| configuration shared by every tenant served by a given Function App | ||
| deployment. | ||
|
|
||
| ## Path safety | ||
|
|
||
| Template keys and storage account names are validated before they reach a blob | ||
| path. Tenant ID and environment are validated at **publish** time by | ||
| `post-kit-publish`; the API-side store does not re-validate them today (see | ||
| **What is not enforced yet** and [#59](https://github.com/singleton-sd/post-kit/issues/59)). | ||
|
|
||
| | Value | Rule | Enforced in | | ||
| | --------------- | ------------------------------------------------------------- | ------------------------------------------------------------- | | ||
| | Template key | `/^[a-zA-Z0-9._-]+$/`, and not the bare dot-segments `.` or `..` | Send handler (`isSafeTemplateKey`), `BlobTemplateStore.load()`, publisher (`assertSafeTemplateKey`) | | ||
| | Tenant ID | Alphanumeric with internal hyphens, no `..` | Publisher (`assertSafeTenantId`) at publish time only | | ||
| | Environment | One of `development`, `staging`, `production` | Publisher (`assertSafeEnvironment`) at publish time only; type system elsewhere | | ||
| | Storage account | `/^[a-z0-9]{3,24}$/` | Publisher (`assertSafeStorageAccount`) | | ||
|
|
||
| The template-key check is deliberately duplicated: the handler rejects an | ||
| unsafe key as `400 INVALID_TEMPLATE` before any storage call, and the store | ||
| re-checks it so the boundary holds even if the store is called from somewhere | ||
| else. Because `/` is not in the allowlist, a key cannot escape its tenant and | ||
| environment prefix, and because bare `.` and `..` are rejected explicitly, | ||
| neither can a dot-segment. | ||
|
|
||
| Recipient addresses go through a basic email-shape check in the handler, and | ||
| `post-kit-email` sanitises header values and rejects CR/LF in header fields, | ||
| so a variable value cannot inject an email header. | ||
|
|
||
| ## Credential and secret handling | ||
|
|
||
| - **Server-side only.** `PostKitClient` is for trusted server code. Browser | ||
| code must never hold a long-lived PostKit token; a public form should POST | ||
| to the consumer's own server route, which then calls PostKit. | ||
| - **Secrets live in Key Vault** (`ssd-global-kv-prod-ae`). Non-secret settings | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: printf '%s\n' '--- repository convention scopes ---'
head -5 /tmp/coderabbit-repo-knowledge/singleton-sd-post-kit-c40aa8a1/*/*.md 2>/dev/null
printf '%s\n' '--- target document ---'
sed -n '110,130p;168,185p' docs/architecture/multi-tenant-security.md
printf '%s\n' '--- repository references to the vault name ---'
rg -n -F 'ssd-global-kv-prod-ae' . --glob '!node_modules' --glob '!dist' --glob '!build'
printf '%s\n' '--- repository identity and document context ---'
git remote -v
sed -n '1,15p' README.md 2>/dev/nullRepository: singleton-sd/post-kit Length of output: 7507 🏁 Script executed: printf '%s\n' '--- authoritative public-repository policy ---'
sed -n '1,140p' docs/github-source-of-truth.md
printf '%s\n' '--- repository instruction context ---'
sed -n '230,275p' AGENTS.md
printf '%s\n' '--- infrastructure and setup evidence ---'
sed -n '1,35p' infra/function-app.bicep
sed -n '105,145p' SETUP.mdRepository: singleton-sd/post-kit Length of output: 12263 🏁 Script executed: rg -n -A18 -B4 'section 7|Public-repository|account/resource|resource identifiers|placeholders' docs/github-source-of-truth.mdRepository: singleton-sd/post-kit Length of output: 1347 Do not publish the Key Vault resource name.
🤖 Prompt for AI Agents |
||
| live in Azure App Configuration or Function App settings. See | ||
| [`SETUP.md`](../../SETUP.md). | ||
| - At startup the API calls `ensureAppConfiguration()`, which reads App | ||
| Configuration and resolves Key Vault references into `process.env` using | ||
|
Comment on lines
+125
to
+126
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/sh
set -eu
rg -n -C 4 '\bensureAppConfiguration\s*\(' apps packagesRepository: singleton-sd/post-kit Length of output: 3671 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/singleton-sd-post-kit-c40aa8a1 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- configuration loader definition ---'
ast-grep outline apps/api/src/config/app-configuration.ts
sed -n '1,115p' apps/api/src/config/app-configuration.ts
printf '%s\n' '--- direct function call sites ---'
sed -n '80,145p' apps/api/src/functions/send.ts
sed -n '1,55p' apps/api/src/functions/contact.ts
printf '%s\n' '--- indirect factory call sites ---'
rg -n -C 5 'BlobTemplateStore\.fromEnv|fromEnv\s*\(' apps/api/srcRepository: singleton-sd/post-kit Length of output: 16327 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- documentation convention ---'
cat /tmp/coderabbit-repo-knowledge/singleton-sd-post-kit-c40aa8a1/conventions/docs.md
printf '%s\n' '--- affected documentation context ---'
sed -n '115,132p' docs/architecture/multi-tenant-security.mdRepository: singleton-sd/post-kit Length of output: 1825 Describe configuration loading as lazy initialization.
🤖 Prompt for AI Agents |
||
| `DefaultAzureCredential`. Explicit environment variables always win over | ||
| the store, and a missing `AZURE_APPCONFIGURATION_ENDPOINT` makes the load a | ||
| no-op (local development and unit tests). | ||
| - Blob Storage access uses `DefaultAzureCredential` — Managed Identity in | ||
| Azure, `az login` locally. No storage connection string or account key is | ||
| required by the send path. | ||
| - CI authenticates to Azure with **OIDC** using repository *Variables* holding | ||
| IDs only. Tokens and `AZURE_CREDENTIALS` in GitHub Secrets are forbidden. | ||
| - `TENANT_KEY_MAP` contains live credentials and is therefore a secret. Never | ||
| commit it, and never paste a real token into an issue, PR, or log. | ||
| - Logging is allowlisted by construction: the logger emits only the declared | ||
| `LogEntry` fields, and the code comments state the rule — no recipient | ||
| addresses, no variable values, no tokens. | ||
| - Caller-supplied `x-correlation-id` values are sanitised to 8–128 characters | ||
| of `[a-zA-Z0-9_-]` before being logged or echoed, so a header cannot inject | ||
| content into a log line. | ||
|
|
||
| ## What is not enforced yet | ||
|
|
||
| State these plainly; do not assume any of them exist. | ||
|
|
||
| - **No per-tenant rate limiting on `POST /emails/send`.** The contact endpoint | ||
| (`POST /contact`) has an in-memory per-IP limiter, but the send endpoint has | ||
| none. A leaked tenant token can be used as fast as the provider allows. | ||
| - **No recipient allowlisting or domain restriction.** `to` only has to look | ||
| like an email address. Any authenticated tenant can send to any address. | ||
| - **No signed webhooks and no delivery-event callbacks.** PostKit returns a | ||
| synchronous `sent` status only; there is no bounce, complaint, or delivery | ||
| notification surface. | ||
| - **No token expiry, rotation, or revocation mechanism.** Revocation means | ||
| editing `TENANT_KEY_MAP`. There is no expiry field, no hashing of stored | ||
| tokens, and no per-token audit trail beyond `tenantId` in the logs. | ||
| - **No per-tenant scoping of the sender identity.** `EMAIL_FROM_ADDRESS` and | ||
| `EMAIL_FROM_NAME` are process-wide, so all tenants on a deployment share the | ||
| configured from address. | ||
| - **No tenant branding store.** The handler merges `TenantBranding` into | ||
| template variables, but the default `resolveBranding` returns `{}`. | ||
| - **No idempotency key.** Retrying a send after a timeout may send twice. | ||
| - **No validation of `TENANT_KEY_MAP` contents.** The JSON is cast to | ||
| `TenantKeyMap` and its `tenantId` / `environment` values reach the blob path | ||
| unchecked, so an operator typo can silently point a credential at an | ||
| unintended prefix instead of failing loudly. Tracked in | ||
| [#59](https://github.com/singleton-sd/post-kit/issues/59). | ||
|
|
||
| Hardening in these areas — additional providers, observability, reliability, | ||
| and security controls — is tracked by | ||
| [#6](https://github.com/singleton-sd/post-kit/issues/6). | ||
|
|
||
| ## Public-repository boundary | ||
|
|
||
| This repository is public. Everything committed or posted here is permanently | ||
| visible. Never place real tenant identifiers, tokens, customer names, or | ||
| account/resource identifiers in code, docs, issues, PRs, or Actions logs — use | ||
| placeholders such as `acme`. The full policy is in | ||
| [`docs/github-source-of-truth.md`](../github-source-of-truth.md) section 7. | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the empty-map failure statement.
When
TENANT_KEY_MAPparsing fails, a syntactically valid but unmapped token returns403 UNAUTHORIZED. A missing, non-Bearer, or emptyAuthorizationvalue still returns401 UNAUTHENTICATEDunder Lines 42-44. Replace “every request” with a narrower statement.🤖 Prompt for AI Agents