Repository navigation
Conversation
The facts proxy built the forwarded query with
{**request.query_params, ...} and the /cloud and /sdk proxies with
dict(request.query_params). Both keep only the last value of a key, so
GET /facts/role_assignments?user=a&user=b asked the backend about user b
alone. Reported in #299.
Forward request.query_params.multi_items() instead, so each key keeps all
of its values in order. In the facts client an explicit query_params
override (return_deleted=True on unassign) still replaces every value the
caller sent for that key.
Co-authored-by: omribz156 <260634988+omribz156@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mapping-rule matching and query attribute extraction read the last value
of a repeated query parameter of the requested URL: ?id=2&id=1 matched a
rule on id=1, ?id=1&id=2 did not, and a rule with ?id={id} checked the
last id. The app behind the URL may read the first value instead, and
then act on a value other than the one checked.
When a parameter a rule reads has more than one distinct value, the URL
now gets no mapping rule at all and /allowed_url answers allow=false with
the reason in debug. Skipping only that rule is not enough: a
lower-priority rule that ignores the parameter would then apply whatever
value the app reads. A rule that no value of the URL could satisfy still
simply does not match, and a parameter repeated with one value counts as
given once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The WARN line for an Authorization header without the Bearer scheme carried the whole header value. It now says only that the scheme is missing. The presented API key is now compared with subtle's ConstantTimeEq instead of `!=`, the same kind of comparison horizon's _token_matches makes with hmac.compare_digest. subtle 2.6.1 is already in Cargo.lock through rustls, so this adds a direct dependency edge but no new crate to the build; it is pinned exactly to the locked version. The test logger now also collects the log lines of the current test thread on request (LogCapture), so a test can check what the middleware logged. New tests cover a lowercase scheme, the right key under another scheme, keys that differ only in length, and the absence of the presented header from every log line on each rejection path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
extract_pdp_api_key answered a header that does not split into a scheme and a token with 401 "bad authz header: <the header>". The detail is now just "bad authz header"; the caller already knows what it sent. The route-level 401 tests for malformed headers now also check that the response does not repeat the header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A query parameter with more than one distinct value stopped the rule
search as soon as any candidate rule read it, so /allowed_url denied
requests that a higher-priority rule matches cleanly. For example, with
a priority-10 rule on ?id={id} and a priority-1 rule on ?tag=x, the
request ?id=1&tag=x&tag=y was denied, although the id rule comes first
whichever tag value the app reads.
Each candidate rule is now kept with its conflict, if any. The rules are
sorted by priority as before (the sort is stable, so equal priorities
keep their order), and only a conflict in the rule that comes first
makes /allowed_url answer not allowed.
Also reword a test docstring to describe the current behaviour.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rule matching takes everything after the first "?" as the query, but
attribute extraction cut the URL at every "?". A "?" inside a query
value made the two read different queries: ?z=1?id=5&id=7 matched a
?id={id} rule and then failed extraction with a KeyError (a 500 from
/allowed_url), and ?id=1?id=2 matched with id "1?id=2" but was checked
as id "1".
Split once in extraction too, so it reads exactly the query the matcher
read.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A regex mapping rule is matched against the raw request URL, so for a repeated query parameter it matched or captured whichever occurrence the pattern reached: a rule on \?id=1 matched ?id=1&id=2 but not ?id=2&id=1, and a named group on id captured the first value. Which parameters a pattern reads cannot be known, so when a parameter has more than one distinct value the rule is also matched against the URL as an app reading the first value, and as one reading the last value, of each such parameter. If the match or a named group differs, the rule is a conflict and goes through the same priority handling as default rules: /allowed_url answers not allowed only when that rule comes first. A pattern that gives the same answer for every reading, such as a catch-all on the path, matches as before. The extra work is two more matches per regex rule on URLs no longer than the original, and only for URLs with such a parameter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LogCapture installs the test logger at Debug, but the logger can be installed only once per test process. When a test that asks for Info runs first, as the health handler tests do, log::max_level() stays at Info and the log macros drop debug and trace records before the capture sees them. A test asserting that no line, at any level, repeats a value would then miss a debug line. LogCapture::start now raises the global maximum to Trace. env_logger keeps filtering what it prints with the level it was built with, so the printed output of other tests does not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
subtle was already in Cargo.lock through rustls, and the image builds with --locked, so the lock already fixes the version. An exact "=2.6.1" requirement would also stop cargo from resolving a future rustls or reqwest release that needs a newer subtle 2.x, since one lock holds one semver-compatible version of a crate. Cargo.lock does not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
/user-permissions dropped ReBAC-derived resource instances after any update to a resource type that other types derive roles from (#341): the permit_rebac.all_roles index removed the child types' derivation rules when the parent type changed, and kept them out until a full data reload. /authorized_users had a related gap when a type gained its first derived role. Both are fixed in permit-opa. The same permit-opa change also builds the updated derivation index as a copy and swaps it in, instead of editing the map a running /user-permissions call may be reading. This moves the pin to that commit. The new commit sits on permit-opa main, so the pin also brings in permitio/permit-opa#53-#57 (query_engine builtins and tests): no version changes; one new test-only indirect module. The commit is not merged yet; re-pin to the merge commit once it is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With PDP_OPA_DECISION_LOG_ENABLED=false the PDP never wrote OPA's config file, and that file is the only place PDP_OPA_PLUGINS reach OPA. The permit_graph plugin then never loaded, and ReBAC user permissions came back empty. Console decision logs (PDP_OPA_DECISION_LOG_CONSOLE, which the Helm chart's logs_forwarder turns on) are set in the same file and were dropped the same way. - The config file is written, and OPA pointed at it through the inline OPA config, when decision log uploads, console decision logs or any OPA plugin is on. - The permit_io service, with the API key it authenticates with, is written only when decision log uploads are on: OPA uses it to upload them and for nothing else. It does not depend on PDP_OPA_BEARER_TOKEN_REQUIRED, which is about callers of OPA. Console decision logs no longer need uploads on. - A different config_file already set in OPAL_INLINE_OPA_CONFIG is still replaced, as it was with decision logs on, and now a warning names both files. - With none of these on and no bearer token required, the inline OPA config is left as it is. Re-implements #274 by omer9564, which tied the service credentials to PDP_OPA_BEARER_TOKEN_REQUIRED instead. Co-authored-by: Omer Zuarets <42326891+omer9564@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The control plane's config overrides set OPA_DECISION_LOG_ENABLED=true for every PDP, and the PDP applied them over its own settings, so a PDP started with PDP_OPA_DECISION_LOG_ENABLED=false still uploaded decision logs. The control plane has sent that key since August 2025; before then a local false was kept. When PDP_OPA_DECISION_LOG_ENABLED is set to a boolean (true, false, 1 or 0, in any case) in the PDP's environment, its value is now kept and that one override is skipped. Without it, the control plane's value is used as before. An empty or other value, which the PDP's settings already read as the default, is named in a warning and the control plane's value is used. The PDP logs which value it uses, and the setting's description says that a value set for the PDP takes precedence. The other overrides are applied unchanged. With uploads off the PDP still writes OPA's config file for its plugins and console decision logs (PER-12470, the previous commit), so turning uploads off does not unload permit_graph or stop the Helm chart's logs_forwarder output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The chart had no way to place the PDP pods, so clusters that taint their node pools or keep workloads on one pool could not schedule it there (#310). The pod spec now takes pdp.nodeSelector, pdp.tolerations and pdp.affinity as given. All three default to empty and render nothing, so existing releases render the same manifests. horizon/tests/test_helm_chart.py renders the chart with helm template and checks that each value lands in the pod spec, on its own and next to the optional logs-forwarder and OpenShift volumes. The pytests job installs a pinned Helm for it; without Helm the tests skip locally and fail in CI. Based on #311. Co-authored-by: cooksec <231643840+cooksec@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The chart always set PDP_API_KEY from a secretKeyRef, to the Secret it creates or to existingApiKeySecret, so the key could not come from something that adds env to the pod, such as a bank-vaults mutating webhook resolving a vault: reference (#300). pdp.userProvidedSecret: true makes the chart create no Secret and set no PDP_API_KEY; PDP_API_KEY comes from pdpEnvs or from the webhook. values.yaml says that pdpEnvs values are stored in the Deployment as plain text, so they should hold a reference the webhook resolves, and a literal key belongs in ApiKey or existingApiKeySecret. Two settings fail the render with a message naming the value: both userProvidedSecret and existingApiKeySecret, since one says to read a Secret and the other to read none, and a userProvidedSecret that is not a boolean, since a quoted "false" is truthy in a template and would drop PDP_API_KEY. The default render is unchanged. The chart tests cover the default, existingApiKeySecret, userProvidedSecret with and without pdpEnvs, the quoted and null values, and both settings together. Based on #315. Co-authored-by: omribz156 <260634988+omribz156@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Publishes pdp.nodeSelector, pdp.tolerations and pdp.affinity (PER-16923) and pdp.userProvidedSecret (PER-16924) together. helm_release.yml only releases when Chart.yaml changes, so both share this one bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Folds in Dependabot's #387. All five pins (tests.yml, release.yml, scheduled-security-scan.yml) move from 7c6b6c3f (v1.24.0) to 221e7f4860634eeb1579e3bd7ca232e577bd1864. #387 labelled that commit v1.26.0, and the v1.26.0 tag does point at it, but so does v1.25.0: it is the v1.25.0 release commit, and its index.js downloads Scout CLI v1.25.0. The v1.26.0 release commit (02c6c79b) has no tag. The comments say v1.25.0, the version that runs. Dependabot reads the current version from the tags on the SHA, not from the comment, and rewrites the comment on the next bump. 1.25.0 makes Scout look inside Debian and Alpine package archives by default, so the docker-scout gate can report packages 1.24.0 did not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Folds in Dependabot's #384. The rev moves from cef0300f (v5.0.0) to 3e8a8703264a2f4a69428a0aa4dcb512790b2c8c, the commit the v6.0.0 tag points to. v6.0.0 needs Python 3.9+ and removes check-byte-order-marker and fix-encoding-pragma; this config uses neither, and every hook it does use passes on the whole tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
opal-common and opal-client 0.9.9 declare Requires-Python <3.13, so they cannot run on the 3.13 image; pyproject.toml holds them at 0.9.6 for that reason. uv ignores Requires-Python upper bounds when it locks, so Dependabot would still open a bump that only the pytests job's pip check on 3.13 rejects. Ignore >=0.9.7, <0.10 for both. A 0.10 release still arrives as a PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every pdp-tester run writes to the same staging environment, and the job had no concurrency group. Overlapping runs failed each other's sync cases: Dependabot PRs 384-386 ran together and each failed a different case, and 387 passed alone right after. The job now joins one repository-wide group with `queue: max`. Runs from PRs, main and releases (which call this workflow) wait their turn in order, up to 100. The default queue would have kept only one waiting run and cancelled it when the next arrived, leaving another PR's required check cancelled. GitHub rejects queue: max with cancel-in-progress, so a started run always finishes. actionlint 1.7.12 does not know the `queue` key (rhysd/actionlint#657), so .github/actionlint.yaml suppresses that one message for tests.yml, with a removal gate. A misspelled concurrency key still fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since the move to uv, GitHub's dependency graph reads the project's own name from pyproject.toml and uv.lock, and matched "horizon" to the unrelated PyPI project of that name, raising that project's Dependabot alerts on this repository. Only the distribution name changes. The import package is still horizon/, and nothing reads the name: the project is virtual (package = false), and the Dockerfile and the pytests job export the lock with --no-emit-project. uv lock only renames the root entry; the exported requirements are identical before and after (79 pins), and the pip check on Python 3.13 still passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The README's build section did not say that the default build clones the private permit-opa repository, or that PDP_VANILLA=true builds with upstream OPA instead (the Makefile maps it to OPA_BUILD=vanilla). Say both, and that an upstream-OPA image cannot evaluate Permit-generated policies, which call builtins only Permit's OPA build has: those need the published permitio/pdp-v2 image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ Cargo Security AuditNo vulnerable crates in |
This was referenced Oct 7, 2026
Open
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| mapping_rules, query.http_method, query.url | ||
| ) | ||
| except ConflictingQueryParameterError as e: | ||
| return _url_not_allowed(query, e) |
| matched_mapping_rule.url, query.url | ||
| ) | ||
| except ConflictingQueryParameterError as e: | ||
| return _url_not_allowed(query, e) |
PDP image vulnerability reportImage: ✅ No CRITICAL or HIGH findings after waivers. TrivyNo CRITICAL or HIGH findings. Docker ScoutNo CRITICAL or HIGH findings. Scanned by Trivy, Docker Scout. Waivers: |
This branch has not been deployed
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
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.
Linear: PER-16920. One PR for the open GitHub items we triaged on 2026-10-07. Each commit covers one item and names its ticket.
Fixes #341
Fixes #299
Fixes #300
Fixes #310
What changed
ReBAC
/user-permissionsafter a resource-type update (#341, PER-16921)/user-permissionsuntil it restarted.tests.ymlandrelease.ymlto that branch./authorized_usersmissing users after a type gains its first derived role.Repeated query parameters through the PDP proxies (#299, PER-16922)
/facts,/cloudand/sdknow forward every value of a repeated parameter, such as?user=a&user=b. Previously only the last value reached the backend.return_deletedstill replaces a client-supplied value.OPA config when decision logs are off (PER-12470, based on #274)
PDP_OPA_PLUGINSgo, sopermit_graphnever loaded.PDP_OPA_BEARER_TOKEN_REQUIRED.config_filefromOPAL_INLINE_OPA_CONFIGnow logs a warning.Decision-log setting precedence (PER-16928)
PDP_OPA_DECISION_LOG_ENABLEDset in the PDP's environment now takes precedence over the value from the control plane, as the docs describe.URL mapping and auth middleware (PER-16927, PER-16926)
/allowed_urlhandles repeated query parameters consistently, for exact, attribute and regex mapping rules.Helm chart 0.0.7 (#310, #300, PER-16923, PER-16924)
pdp.nodeSelector,pdp.tolerationsandpdp.affinity. Based on Add nodeselector, affinity, and toleration configuration options to the helm chart #311 by @cooksec.pdp.userProvidedSecret: the chart creates no Secret and doesn't setPDP_API_KEY, so it can come frompdpEnvs(for example through a Vault webhook). Based on feat(helm): support user-provided API key env #315 by @omribz156.userProvidedSecretandexistingApiKeySecret, or a non-booleanuserProvidedSecret, fails the render with a clear message.CI and packaging (PER-16929, PER-16930, PER-16931, PER-16934)
concurrencywithqueue: max, so concurrent runs no longer share the staging env and no other PR's required check gets cancelled.221e7f48, labelled v1.25.0. Upstream's v1.25.0 and v1.26.0 tags point at the same commit, and it downloads Scout 1.25.0. Replaces deps: bump docker/scout-action from 1.24.0 to 1.26.0 in the minor-and-patch group #387.Requires-Python <3.13).horizontopermit-pdp; the import package stayshorizon. This stops Dependabot matching OpenStack Horizon advisories against this repo.PDP_VANILLA=true) and what that build can't do.How tested
pytest horizon/tests: 845 passed (704 before).cargo test --all --locked: 219 passed. Every new test was shown to fail without its fix.prek run --all-filespasses all 19 hooks (ruff, ty, clippy, fmt, uv.lock, waiver parity). actionlint and zizmor are clean.permitio/pdp-v2:latestas the control.resource_types/<type>update, the control lost derived instances in 6 of 6 runs; the fix lost none.GETrequest hasuserfilter array collapsed to single value #299: a role-assignments list with two users returned one user on the control and both on the fix, through each of/facts,/cloudand/sdk.permit_graphloads and ReBAC user permissions are returned. With logs on and bearer optional, upload credentials are present.helm lintandhelm templateacross all value combinations, and a throwaway k3d cluster. Main's chart leaves the pod Pending on a tainted node; with this branch it schedules and becomes Ready, including withuserProvidedSecretpluspdpEnvs.check_aiofiles_override.pypasses, and the PDP decides the same as the released image.Before merging
tests.ymlandrelease.ymlto its merge commit.PDP_OPA_DECISION_LOG_ENABLED=falsewill stop uploading decision logs.Credits
@cooksec (#311), @omribz156 (#314, #315), @omer9564 (#274). Each is credited with
Co-authored-byon the matching commit.🤖 Generated with Claude Code