4 Commits
Author SHA1 Message Date
Julia Ogris f433c29963 appresource: Add rule and decision types (#195)
Export-Source-Commit: 9ac304b68a
2026-08-25 11:35:10 -05:00
Julia Ogris 4c3cb314ec app: Add audit event for role v9 HTTP deny (#194)
Export-Source-Commit: d7bb0d36d7
2026-08-25 11:35:10 -05:00
Julia Ogris b887107e7f appresource: Add path tokenizer and validator (#68795)
* appresource: Add path tokenizer and validator

Create the `lib/appresource` package with the request-path tokenizer
and wire-form validator, promoted from the sketch branch.

`Tokenize` validates a raw request path and splits it into segments
without decoding the bytes it forwards. A separate decode is used
only as a validation view. Tokenize rejects any byte outside the RFC
3986 path set less ";", admits the encoded separator `%2F`/`%2f` and
percent-encoded non-ASCII content as the only escapes, rejects "."
and ".." segments and "//" on a view that also decodes the encoded
separator, and requires each segment, decoded once, to be valid
UTF-8, NFKC-stable, and made of graphic runes. Together these
prevent the canonicalization and fold bypasses where an upstream
decodes or normalizes a path onto a segment the matcher never saw.
Paths over 8 KiB are rejected.

The package is a leaf so the app agent, role validation in
`lib/services`, and tctl can all import it. Node matching, captures,
and the encoded-slash grammar follow in later changes.

* appresource: Validate path content per segment

The NFKC and graphic-rune checks ran on the whole path body with the
encoded separator `%2F` kept literal. A combining mark placed right
after an encoded slash then composed with that literal `F` (`F` plus
U+0307 forms `Ḟ`), so a legitimate path was rejected as not
NFKC-normalized even though its real decoded content is normal.

Move the content checks into `validateDecoded` so they run per segment
on the decode-for-validation view, where `%2F` is a separator. An
encoded slash and a raw `/` now validate the same way, and the merge
drops a second `decodeSeparators` pass and split. Pin the
non-breaking-space reject case to the NFKC rule, since the graphic
check would otherwise let it pass for the wrong reason.

* appresource: Address tokenizer review feedback

Reject a segment that starts with a combining mark. The mark pairs
with the previous rune, and net/http.ServeMux still splits on the
literal slash, so upstreams may disagree on the segment boundary.
RFC 5891 imposes the same rule on the start of a domain label.

Re-add the rapid property tests. Unlike the fuzz target, which only
replays its seed corpus under go test, they exercise the tokenizer
invariants on every run. Register the tokenizer fuzz target in
fuzz/oss-fuzz-build.sh and pin the encoded hash %23 (#) as rejected.

* appresource: Allow the encoded space %20 in paths

Allow %20 as content, alongside the non-ASCII escapes. A space has no
raw form in a URL path, so the escape is its only representation, and
rejecting it makes resources unreachable with no workaround. Jenkins
serves job names with spaces at /job/My%20Job/lastBuild, and SharePoint
serves its default library at Shared%20Documents. A space cannot decode
into a separator, so it needs neither a rule-global grant nor a
per-segment pattern, unlike the encoded separator %2F.

Reject a segment that starts or ends with a space. Windows and IIS trim
a trailing space from a segment, so ".. " would resolve as ".." and
"secret " would resolve as "secret", escaping a carve-out. That is the
same fold-to-a-different-segment problem the other content checks
prevent, reached by trimming rather than normalization.

Reject a segment of only dots and spaces, which folds the former "."
and ".." equality check into one rule. A normalizer that strips spaces
or a single trailing dot resolves ". ." and "..." the same way.

Permit U+0020 alone in isGraphicRune, never the Zs category. U+1680
ogham space mark, U+2028, U+200B and U+0085 are NFKC-stable, so the
graphic-rune check is the only place that rejects them.

* appresource: Reject a segment's trailing dots

Reject a segment whose trailing run of dots and spaces contains a dot.
IIS and Windows trim trailing dots and spaces from a path component,
so an upstream would resolve `secret.` and `secret%20.` as `secret`,
a segment the matcher never saw. The edge-space rule already closes
this trim vector for the space; close it for the dot the same way. A
leading dot stays allowed, so `/.well-known` remains reachable.

Fold the escape predicate shared by `validatePercentEscapes` and
`decode` into `isAllowedEscape`, so the two functions visibly accept
the same escape set. Clip paths and segments to 256 bytes in error
messages, because a rejected path can be kilobytes long and the
message survives into logs. Pin the acceptance of a combining mark
riding a trailing interior space, which ends the segment with a mark
rather than a space byte, so no upstream trim fires on it.

* appresource: Reject a leading conjoining jamo

Extend the leading-mark check with the NFKC boundary property. A
conjoining Hangul jamo is a letter, not a mark, so unicode.IsMark
misses it, yet it composes onto the character before it the same way.
A segment of "ᆨ" after a segment ending in "가" joins to "각" once an
upstream drops the separator, which is the fold the check prevents.

Render an illegal byte as a one-byte slice instead of converting it to
a rune, so a raw 0xC3 prints as "\xc3" rather than "Ã".

* appresource: Simplify godocs and merge byte checks

Drop the numbered rule list from the Tokenize godoc, along with the
terms NFKC-stable, graphic rune, and conjoining Hangul jamo that a
caller does not need in order to call the function correctly. Say what
to pass, what comes back, and which paths are rejected, and keep the
precise rules on the reject helpers next to the code that enforces
them. Add an inline comment where the leading mark check needs both
unicode.IsMark and the NFKC boundary property, since neither covers
the other.

Rename lengthCap to maxPathLength, which reads better at the call
site, and merge validatePathBytes and validatePercentEscapes into
validateRawBytes so the raw path is walked once rather than twice.
Both loops were byte-level and neither depended on the other having
finished, and the hex digits the escape branch skips are alphanumeric,
so they were always legal path bytes anyway.

Swap the formatted testify variants in the property tests for the
plain ones, which take the same message and arguments.
2026-08-07 22:31:37 +00:00
Julia Ogris 2c92419557 appresource: Compile and evaluate where clauses (#68796)
* appresource: Compile and evaluate where clauses

Create the leaf package `lib/appresource` with the predicate language
for HTTP app access. Carry over the where environment from the sketch
branch `julia/app/policy-matcher-sketch`. That environment covers the
`user.name`, `user.roles`, `user.traits`, and `request.method`
bindings, the `true` and `false` constants, and the list and string
helpers (`set`, `contains`, `lower`, `upper`, `has_prefix`,
`has_suffix`, `has_substring`).

Export `CompileWhere` and `Where.Evaluate` as the entry point, so a
caller can compile a where clause and match it against a request and a
caller identity. Keep the parser unexported. Name the function for the
clause it compiles, because a follow-up PR adds a second entry point
for the desugared expression form. Pass the evaluation environment as
one `Env` struct, so the `vars.` capture namespace can be added as a new
field without changing the `Evaluate` signature.

Validate the environment before evaluation and reject any method
outside the canonical upper-case HTTP method list, so a case-varied
method can neither slip past a negated comparison nor match a clause
while the backend receives a different spelling.

Aggregate `trace.BadParameter` with the parse error in `CompileWhere`,
matching the sibling path tokenizer, so a caller can tell a malformed
clause in a role from an internal fault without losing the parse error
chain. Report no match when evaluation fails, so a caller that drops
the error denies rather than allows.

Bind `true` and `false` explicitly, because `typical` has no boolean
literal, so a bare literal would otherwise fail as an unknown
identifier.

Leave the `allow_code` and `deny_hint` audit wrappers to the PR that
adds the rule types. Both wrappers record against the rule that
matched, and checking that a wrapper sits where its record can be
attributed correctly needs the rule types to define what one rule's
expression is. Compile clauses over identity and method only. Leave
path matching and the `vars.` capture namespace to follow-up PRs as
well.

* appresource: Use net/http method constants

Replace the hand-written method strings in `validMethods` with the
`net/http` constants, so the list pins the canonical spellings instead
of repeating them by hand.

* appresource: Add property-based where clause tests

Add rapid property tests over generated clauses and environments. The
properties are that `CompileWhere` never panics on arbitrary input,
that a clause and its negation never agree, that any method outside
the canonical list is denied for every clause, that `contains` over
`set` matches `slices.Contains`, and that evaluation is
deterministic.

* appresource: Address review comments

Add `NewEnv` so the canonical HTTP method check runs once at the request
boundary. A `where` clause is optional on a rule, so a rule with only
`methods` and `paths` never builds a `Where` and never reached the check,
which meant adding a no-op `where: "true"` to an otherwise identical rule
changed the answer for a request spelled `get`. `Evaluate` repeats the
check, so an environment a caller assembled itself still cannot authorize
such a request.

Rename `validateEnv` to `validateMethod`, because the request method is
the only thing it inspects.

Record why `CompileWhere` builds an aggregate rather than wrapping. The
aggregate classifies the result as `BadParameter` for the caller and keeps
`typical`'s typed error for `errors.As`, and `Parse` does not return a
`BadParameter` for every failure.

Drop the claim that `contains` matches the role where-clause function of
the same name. `predicate.Contains` type-asserts its first argument to
`[]string` and returns false for a string, while `typical` coerces a
string to a one-element list, so the same clause text disagrees across
the two languages.

State that a trait key the identity does not have reads as an empty list,
since a mistyped key under a negation then matches every caller. State
that the `Evaluate` result is only meaningful when the error is nil, and
that sharing an `Env` across evaluations requires the caller not to
mutate its slices or map.

Cover the clauses that type-check to something other than a bool,
operator precedence, and an identity with no traits. Widen the
`strconv.Quote` round-trip generator to arbitrary strings, since the
restricted alphabet never exercised the escaping the property is about.
Rename the tests that did not name the behaviour they check, and check
that `CompileWhere` never returns a `Where` with a nil expression.

* appresource: Address review feedback on tests

Draw a well-formed clause for half the cases in
`TestCompileReturnsValueOrError`. Arbitrary strings essentially never
parse, so the success branch never ran and the test asserted nothing.
Measured 0 successful compiles in 20000 draws of `rapid.String()`, against
2496 in 5000 draws of the mixed generator.

Assert that a compile failure returns no `Where`, which no other test
checks, since `TestInvalidWhereClause` discards the value. Keep the
generator choice in a variable so a well-formed clause that fails to
compile is a test failure rather than an early return.

Drop `TestEvaluateDeterministic`. Evaluation holds no state and no map
iteration reaches the result, so the property cannot fail. The cached
parser path it exercised is already reached by the other property tests,
which compile the same clause more than once.

Use `identifierText` directly rather than aliasing it per call, now that
the generator is built once for the package. Say why the two assertions
after the `TestEnvBindings` table sit outside it, since they cover a
non-GET method and a nil traits map that the fixed request and identity
cannot express. Check the error class in `TestUnsupportedMethodRejected`
as `TestNewEnv` already does.
2026-08-04 23:14:08 +00:00