Invental/Review lessons/Trust boundaries the server must hold

18 lessons · 27 checks

Trust boundaries the server must hold

A check on the main path is easy to see. The holes are on the side paths: the endpoint the UI never calls, the field only the server should set, the parser that runs on someone else's input. Each lesson is one boundary that has to hold on the server.

Major

Existence checks can leak restricted data

“Does any secret have this value?” is itself a read of secret data. Gate it at least as tightly as reading the value, or it becomes a guessing oracle across permission boundaries.

Read the lesson → Major

An empty list must never mean everyone

Never let an empty list mean “everyone” on a path that sends secrets. When the selection is gone, fail closed and make the user choose again.

Read the lesson → Major

Take audit scope from validated input

Take the scope of an audit decision from the authorisation check, or from validated input, never from raw request fields. Otherwise the person being audited can choose the settings that judge them.

Read the lesson → Major

Start-up scripts must not be world-writable

Anything a container runs later (hooks, entrypoints, cron scripts) must not be writable by the users who run inside it. chmod 777 in a Dockerfile turns “install script” into “anyone's script”.

Read the lesson → Minor

Quote every value in generated shell scripts

Shell scripts built from template strings need every interpolated value quoted, and quoting has a cost: "~/x" stops meaning your home folder. Decide per variable, and test with a space and a tilde.

Read the lesson → Major

Escape secrets before you mask them in logs

Masking secrets is string replacement on untrusted input: escape the values, drop empty ones, mask structured log events before formatting, and check every field, not just the message.

Read the lesson → Major

A POST that exports data is still a read

Don't derive the permission action from the HTTP verb: a POST that exports data is a read. Pass the action explicitly at every route that doesn't follow REST.

Read the lesson → Major

User-chosen keys need a null-prototype object

Any object keyed by user input (URLs, names, ids) and loaded from JSON can be prototype-polluted: build it on Object.create(null) or use a Map.

Read the lesson → Major

Shared tables need row rules in code

When two features share one table, enforce “who may touch which rows” in code. Then check every read path that skips your service (joins, counts) as well: editable permission presets don't stop new or retyped rows.

Read the lesson → Major

A disabled toggle is not a permission

A disabled switch is not a permission rule: re-check the invariant in the server action. And revoke tokens based on what changed (old grant vs new), not just on what the new grant lacks.

Read the lesson → Major

Lazy regexes on untrusted input

Any parser that runs on inbound email, webhooks or uploads takes attacker-controlled input: a lazy regex over “open tag … close tag” can go quadratic when the close tag never comes.

Read the lesson → Major

Detached DOM nodes still fetch their src

“Detached” is not “inert”: an <img> created in the live document fetches its src immediately, so build untrusted HTML in a separate document (<template> content or DOMParser) before you set any URL.

Read the lesson → Major

Pin an approval to the exact version shown

An approval is only meaningful if it names an immutable thing: pin the exact version id into the approved action, and put the approval gate in the tool's execution path, not in the model's instructions.

Read the lesson → Major

Redact streamed secrets across chunk edges

Redacting a stream is not “redact each chunk”: keep back any tail that could be the start of a secret, and never cut output in the middle of a match.

Read the lesson → Major

Give the CI push token to one step only

A token persisted by checkout is readable by every later step, including install scripts from the code under review; give the credential only to the one step that pushes.

Read the lesson → Major

Never accept fields only the server sets

When callers can pre-fill parts of a state machine, check each pre-filled piece against the same invariants the machine counts on (reachability, iteration). Also shape the input so that duplicates and engine-only flags can't be expressed at all.

Read the lesson → Major

Scope what an SSO email assertion unlocks

When an external identity provider vouches for an email, scope what that vouching unlocks. A per-workspace assertion should not flip a global “verified” flag on an account that has its own password.

Read the lesson → Major

Validate templates after variables are filled

Sanitise and derive after template variables are filled in, not before: a URL check that runs on {{url}} will either reject a good link or approve a placeholder. And never guess a data format on a shared write path; convert where the format is known.

Read the lesson →

The checks, with the lesson behind each

Do any of your "exists / duplicate / match" endpoints tell users about records they are not allowed to read?+
"Does any secret have this value?" is itself a read of secret data. Gate it at least as tightly as reading the value, or it becomes a guessing oracle across permission boundaries.
In your app, can a filter that removes every recipient quietly turn into "send to everyone"?+
Never let an empty list mean "everyone" on a path that sends secrets. When the selection is gone, fail closed and make the user choose again.
Can a user change whether their own failed action gets logged by adding a parameter to the request?+
Take the scope of an audit decision from the authorisation check, or from validated input, never from raw request fields. Otherwise the person being audited can choose the settings that judge them.
Does any script that your container runs at start-up live in a folder that a non-root user can write to?+
Anything a container runs later (hooks, entrypoints, cron scripts) must not be writable by the users who run inside it. chmod 777 in a Dockerfile turns "install script" into "anyone's script".
If a user's path contains a space, does every shell command your app generates still work?+
Shell scripts built from template strings need every interpolated value quoted, and quoting has a cost: "~/x" stops meaning your home folder. Decide per variable, and test with a space and a tilde.
If one of your secrets contains a $ or a ., is it still masked in every log line and every structured log field?+
Masking secrets is string replacement on untrusted input: escape the values, drop empty ones, mask structured log events before formatting, and check every field, not just the message.
Do any of your non-CRUD POST routes (export, search, preview) run permission checks as if they were creates?+
Don't derive the permission action from the HTTP verb: a POST that exports data is a read. Pass the action explicitly at every route that doesn't follow REST.
Does your CLI or service store anything in a plain object whose keys come from users, and have you tried the key __proto__?+
Any object keyed by user input (URLs, names, ids) and loaded from JSON can be prototype-polluted: build it on Object.create(null) or use a Map.
If a table now holds rows for two features, can a user with access to one feature reach the other's rows through a join, a count, or a type change?+
When two features share one table, enforce "who may touch which rows" in code. Then check every read path that skips your service (joins, counts) as well: editable permission presets don't stop new or retyped rows.
For every toggle your UI disables, does the server action reject that same change if called directly?+
A disabled switch is not a permission rule: re-check the invariant in the server action. And revoke tokens based on what changed (old grant vs new), not just on what the new grant lacks.
Does any regex in your app run on raw HTML or text from outside users, and have you timed it on 1 MB of unclosed tags?+
Any parser that runs on inbound email, webhooks or uploads takes attacker-controlled input: a lazy regex over "open tag … close tag" can go quadratic when the close tag never comes.
Does your app ever set src on an element built from someone else's HTML before sanitising it, even off-screen?+
"Detached" is not "inert": an <img> created in the live document fetches its src immediately, so build untrusted HTML in a separate document (<template> content or DOMParser) before you set any URL.
When a user approves an AI agent's action, is the approved payload frozen, or can it change before it runs?+
An approval is only meaningful if it names an immutable thing: pin the exact version id into the approved action, and put the approval gate in the tool's execution path, not in the model's instructions.
If you mask secrets in streamed logs or LLM output, what happens when a secret arrives split across two chunks?+
Redacting a stream is not "redact each chunk": keep back any tail that could be the start of a secret, and never cut output in the middle of a match.
In your CI, does any step that installs or runs PR code run after a checkout that persisted a write token?+
A token persisted by checkout is readable by every later step, including install scripts from the code under review; give the credential only to the one step that pushes.
Can a client of your API send a field that only your server is supposed to set, and does the server keep it?+
When callers can pre-fill parts of a state machine, check each pre-filled piece against the same invariants the machine counts on (reachability, iteration). Also shape the input so that duplicates and engine-only flags can't be expressed at all.
If one tenant's admin connects an SSO provider that asserts any email, can that unlock an account in another tenant?+
When an external identity provider vouches for an email, scope what that vouching unlocks. A per-workspace assertion should not flip a global "verified" flag on an account that has its own password.
Where your app fills templates (emails, workflow actions, AI prompts), does validation or escaping run on the filled-in value, or on the placeholder?+
Sanitise and derive after template variables are filled in, not before: a URL check that runs on {{url}} will either reject a good link or approve a placeholder. And never guess a data format on a shared write path; convert where the format is known.

More checks from the same reviews

Yes/no questions from other reviews on this theme. Each one is quick to answer for your own app.

  1. Can anyone holding one of your upload URLs replace a file that is already stored?
  2. Does your app decrypt or fetch sensitive data before the permission check that might reject the request?
  3. Does every endpoint that triggers a paid or restricted action enforce the same gate as your main create endpoint?
  4. When you import a zip or folder from a user, do you check that every resolved path stays inside the extraction directory on every OS you support?
  5. Does any of your permission code turn an access filter into a yes/no with a truthiness check?
  6. When you add a new endpoint next to an old one, have you listed which of the old one's checks it must also run?
  7. When a list's filter changes (team, group, tenant), can the UI show and edit rows from the previous filter while loading?
  8. For each admin-only list or delete endpoint, would an admin of organization A get rows that belong to organization B?
  9. If you revoke a user's permission today, do their already-scheduled jobs or messages still run tomorrow, and do your docs say which?

Want a senior reviewer to run these checks on your code? Security review of an AI-built JavaScript or TypeScript app →

Want these checked on your code?

We run reviews like these on real codebases. Tell us what you are building and we confirm scope before anything starts.

— Get in touch
hi@invental.co ↗
— Or
— What to include

A link or short description of the code or system, what you want checked, your stack, and when you need the answer. No repository access is needed until scope is agreed.

— Or leave a note
We reply within one business day.