Q_Utils::sign() throws without Q/internal/secret; signature() keeps the local fallback; install.php checks the secret - #55
Open
zattak1 wants to merge 2 commits into
Conversation
…ecret is not configured
Q_Valid::signature() returned TRUE for every payload when no secret was
available:
if (!isset($secret)) {
return true;
}
It is a validator, and it read "no key" as "valid". Everything built on it is
an internal gate -- handlers/Q/config/validate.php, which is the check on
Q_Config_post() before it dispatches to Q_Config::setOnServer() and
clearOnServer() with a client-supplied filename and data; Streams::invites();
Media/callCenter -- so an install with no Q/internal/secret accepted an
unsigned POST that writes config files onto the machine, while looking
healthy.
This is only the ACCEPTING side. Producing a signature (Q_Utils::signature())
is untouched here and still falls back to a machine-local key; that policy is
a separate change. Accepting an unsigned request must fail closed regardless
of what the producing side does.
Q_Utils::requireInternalSecret() is new and additive: it returns the
configured secret, and treats the empty string and the "TODO: CHANGE TO SOME
RANDOM STRING, ..." literal that local.sample/app.json ships as unconfigured.
That literal is a fixed value published in every copy of this repository, so
an install that keeps it is not weakly configured -- it holds a secret every
attacker already has, and can therefore sign.
All three in-tree call sites pass $throwIfInvalid = true, so the rejection
surfaces as the existing Q_Response::code(403), with Q_Exception_MissingConfig
naming the key at fault rather than blaming the caller for a local
misconfiguration.
platform/classes/Q/Utils.js carried the same fallback in Utils.validate(),
whose docblock advertised "Returns true if secret is empty". It now returns
false with a console warning. Utils.signature() and Utils.sign() are untouched.
Behaviour with a secret configured is unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he local fallback; install.php checks the secret Splits the signing side of the fail-closed change along the line between producing a signature and accepting one. - Q_Utils::sign() throws Q_Exception_MissingConfig when no secret is passed and Q/internal/secret is not configured (or is the shipped "TODO:" placeholder). It hands out an outbound credential, and signing that with a guessable machine-derived key is worse than not signing at all. - Q_Utils::signature() keeps generateLocalSecret() as its fallback. It is called from Q_Session::isValidId() on every request, from Q_Bootstrap::validateEarly(), before there is any sensible error path; a throw there would turn an install missing the secret into a 500 at the door, discovered by the first visitor after a deploy. Degrading to a machine-local key there lets nobody in: accepting a signature from outside never degrades (Q_Valid::signature(), previous commit). - generateLocalSecret() memoizes in a static. Its inputs are fixed for the life of the process, and it is now on the per-request path several times on an install without a configured secret; previously it re-read /etc/machine-id (or spawned `reg query` on Windows) on every call. Output is unchanged. - Q_Plugin::installApp() checks the secret up front and fails with the config path in the message. That is where an operator can act on it. - Same split on the Node side: Utils.sign() throws, Utils.signature() keeps the fallback, generateLocalSecret() memoizes. Behaviour with a secret configured is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 8, 2026
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.
The producing half of #40, reshaped along the split proposed there: accepting an unsigned request fails closed always (that's #54, which this PR is based on — it will show that PR's commit until it merges); producing a signature, or verifying an artifact we issued ourselves, may degrade to a machine-local key without letting anyone in.
What changes
Q_Utils::sign()throwsQ_Exception_MissingConfigwhen no secret is passed andQ/internal/secretis not configured (or is the shipped placeholder). It hands out an outbound credential; signing that with a guessable key is worse than not signing.Q_Utils::signature()keepsgenerateLocalSecret()as its fallback. It runs fromQ_Session::isValidId()on every request, fromQ_Bootstrap::validateEarly(), before any sensible error path — a throw there is a 500 at the door discovered by the first visitor after a deploy. The comment at the fallback says why it is allowed to degrade and where the missing secret is meant to be caught instead.generateLocalSecret()memoizes in astatic. Inputs are fixed for the life of the process; it now sits on the per-request path several times on an unconfigured install and previously re-read/etc/machine-id(or spawnedreg query) each call. Output unchanged.Q_Plugin::installApp()checks the secret up front and fails with the config path in the message (<APP_LOCAL_DIR>/app.json→Q/internal/secret), so a missing or placeholder secret is found where an operator can act on it.Q/Utils.js:Utils.sign()throws,Utils.signature()keeps the fallback,generateLocalSecret()memoizes.One thing #40's description raised that this shape leaves in place, stated so it isn't lost:
generateLocalSecret()differs between the PHP host and the Node host, so on an install without a configured secret, PHP↔Node internal signing never verifies. With this PR that is caught at install time rather than at runtime, which is the intended place for it.Behaviour with a secret configured is unchanged.
🤖 Generated with Claude Code