Repository navigation
Merging in Hongwei's PR #2905 dynamic resource doc java support + - #2929
Merged
Merged
Conversation
method_body can now be Scala (default, unchanged) or Java, mirroring the existing precedent already used by ConnectorMethod/DynamicMessageDoc. - add programming_lang field to JsonDynamicResourceDoc / DynamicResourceDoc.Lang - compile Java method_body via the JSR-223 java engine into a native Http4sEndpointIO, reusing the same javax.tools.JavaCompiler backend as createJavaFunction - validate the actual compiled Java class (not its Scala wrapper) against dynamic_code_compile_validate_dependencies before it is ever invoked -- the wrapper alone can't see into a compiled Function reference (typically a synthetic lambda class) the way it can for a Scala closure - convert Java-native return values (Map/List/String/number/boolean) to JValue directly, since Extraction.decompose only reflects Scala types and silently drops a raw java.util.Map's entries - reject unsupported programming_lang values with a 400 before attempting compilation - thread programming_lang through DynamicResourceDocsEndpointGroup, which recompiles every registered doc when serving it -- without this a stored Java doc would silently be recompiled as Scala and fail at request time
POST /management/dynamic-resource-docs/validate constructed CompiledObjects without the doc's programming_lang, so validating a Java-language method_body silently tried to compile it as Scala and always reported a (misleading) CompilationError, and the success message hardcoded "valid Scala" regardless of the actual language.
…elper SonarCloud flagged the "dynamic-resource-docs" URL segment repeated three times across DynamicResourceDocJavaTest's three scenarios.
…urce docs DynamicResourceDocJavaTest's role-gated scenario duplicated the same 17-line 401/403/200 assertion block already in DynamicResourceDocTest, which SonarCloud's new-code duplication gate flagged. Both now share V400ServerSetup.assertRoleGated401Then403Then200.
Kept in sync with frozen_type_meta_data (regenerated by the previous commit) via code.util.FrozenMetaDataText -- FrozenMetaDataTextTest fails when the two disagree.
Two bugs made createJavaHttp4sEndpoint's validation a no-op in practice, discovered by manually enabling dynamic_code_compile_validate_enable against a real Java method_body that calls a non-whitelisted OBP method: - Javassist's LoaderClassPath reads a class's bytecode via classLoader.getResourceAsStream(...), but the compiler's ch.obermuhlner.scriptengine.java.MemoryClassLoader only overrides loadClass() and never exposes compiled bytes as a classpath resource. getDynamicCodeDependentMethods silently found nothing to check no matter what the Java code called. Fix: read the bytes from the classloader's private byte map and register them with Javassist directly via ByteArrayClassPath. - Once dependency lookup worked, a rejection was still swallowed: JsonResponseException never sets a Throwable message, so wrapping the validation call in Box.tryo turned it into Failure(null, Full(theException), Empty), and DynamicEndpoints.scala's `case Failure(msg: String, ...)` pattern fails to match a null msg -- silently falling through to a generic "compiled code return nothing" error and discarding the real rejection reason. Fix: run validation outside any Box.tryo, mirroring how the Scala path's CompiledObjects.validateDependency() (also never tryo-wrapped) already lets the exception propagate uncaught. Manually verified end-to-end: a Java method_body calling code.api.util.APIUtil.getPropsValue (not on the dependency whitelist) is now rejected with 400 OBP-40046, naming the exact forbidden call.
DynamicUtil.Validation's dependency whitelist and Constant.SHOW_USED_CONNECTOR_METHODS were final vals, computed once when first touched (in practice during server boot, before any test scenario runs). setPropsValues -- the standard per-scenario props override used throughout this test suite -- mutates Props.lockedProviders, which only reaches code that re-reads Props on each call; it cannot un-freeze an already-computed field. That made this security path untestable without a real server restart, which is how the earlier manual verification (see prior commit) had to prove it. Changed the whole SHOW_USED_CONNECTOR_METHODS / Validation dependency chain from val to def so it re-evaluates from live props on each call. This costs nothing extra in production: the one expensive step, DynamicUtil.compileScalaCodeUnchecked, is already memoized by the exact source string, so re-evaluating is a cache hit unless the underlying props value actually changed. DynamicResourceDocJavaSecurityValidationTest exercises the resulting create-time HTTP path directly: a Java method_body calling code.api.util.APIUtil.getPropsValue (not on the dependency whitelist) is rejected with 400 OBP-40046, naming the exact forbidden call -- the same assertion the manual verification made by hand.
createJavaHttp4sEndpoint memoized the whole Box[Http4sEndpointIO] -- including whatever Validation.validateDependency decided -- keyed only by the exact method_body string. Once dynamic_code_compile_validate_enable and the whitelist became live-reloadable (previous commit), this became a bypass: compile a Java source while validation is off, then turn validation on and tighten the whitelist, then resubmit the identical source in a new create/update call -- the cached Full(...) from the first, unvalidated compile is returned directly, and validateDependency never runs a second time. Split the memoization so only the actual javax.tools.JavaCompiler invocation is cached (deterministic given the source, and the one genuinely expensive step); dependency validation now runs on every call, unmemoized, exactly like the Scala path's CompiledObjects.validateDependency() already does. Also corrected an overclaiming comment: the val-to-def change makes Validation.allowedRuntimePermissions itself always current, but Sandbox.sandbox(bankId) separately caches the whole Sandbox it builds per bankId, so a sandbox built before a permissions-prop change keeps the old snapshot. Left that cache alone -- SecurityManager enforcement is already a no-op on this JDK (JEP 486), so neither the stale nor the fresh permission list is actually enforced either way. DynamicResourceDocJavaSecurityValidationTest gains a regression scenario reproducing the exact bypass sequence: compile the malicious source once with validation off, enable strict validation, resubmit the identical source under a new doc, and assert it is still rejected.
enableStrictValidation() wrapped setPropsValues with an expression body (def ... = setPropsValues(...)), which .github/scripts/ check_test_isolation.py's brace-based scanner does not recognize as a safe "helper" scope -- it only classifies a def as safe when it finds an opening brace immediately following the def name. The call was flagged as running at class-instantiation time and failed CI's lint step before compilation even started. Verified locally with python3 .github/scripts/check_test_isolation.py.
…r_methods getDynamicCodeDependentMethods and APIUtil.getDependentMethods were both gated by SHOW_USED_CONNECTOR_METHODS, an unrelated introspection/reporting toggle that defaults to false. This meant dynamic_code_compile_validate_enable alone did nothing: with the reporting prop left at its default, the bytecode scan always returned an empty dependency list, so every dynamic-code call passed the whitelist check regardless of what it actually invoked. Add a force parameter that Validation.validateDependency sets to true, making security validation controlled solely by dynamic_code_compile_validate_enable as documented. The existing caller in DynamicCompileEndpoint keeps its prior behaviour unchanged.
createJavaHttp4sEndpoint re-registered the compiled class's bytes into the shared Javassist ClassPool on every call, including compile-cache hits. ClassPool.appendClassPath has no dedup, so a long-running process serving the same dynamic endpoint repeatedly (e.g. on each resourceDocs-list rebuild) grew the ClassPool's classpath chain without bound. Move the registration inside the compile memoization block so it runs exactly once per distinct source string, tied to the same cache lifetime as the compiled class itself.
…lumn
getJsonDynamicResourceDoc passed Lang.get straight through as a
constructor argument. Rows created before the Lang column existed have a
genuine SQL NULL there, and an explicit null argument bypasses
JsonDynamicResourceDoc's own "Scala" default -- that default only applies
when the argument is omitted. Legacy rows therefore reported
programming_lang as null/empty instead of the documented default.
Fall back explicitly with Option(...).filter(isNotBlank).getOrElse("Scala"),
matching the pattern already used for the other nullable text fields in
the same method. Added a regression test that forces a genuine NULL via
raw SQL (bypassing the ORM, which always writes "Scala") and asserts GET
still reports "Scala".
… dynamic resource doc Two compounding bugs, both required to reproduce and both required to fix, found by running the feature against a real packaged jar and a real Postgres database instead of the in-process test harness: 1. Every Java method_body implements Supplier<Function<Object[], Object>> per convention. javac always erases that generic Supplier.get() to a synthetic bridge method (Object get()) whose body just invokevirtual-calls the real, properly-typed get() -- an ordinary same-class call. The dependency scanner's same-class exemption only recognised a Scala-specific mangled-name convention, so this call was treated as a dependency on a forbidden method: the dynamically-compiled class lives under the OBP-owned code.* package, but its randomly-generated name can never appear in a static whitelist. 2. CompiledObjects.validateDependency() re-validated this.partialFunction a second time after DynamicUtil.createJavaHttp4sEndpoint had already validated the real compiled Java class internally. For Java, this.partialFunction is OBP's own Http4sEndpointIO wrapper, not user code, and its bytecode legitimately calls internal OBP helpers (javaValueToJValue, logger, CustomJsonFormats.formats, compactRender) that were never meant to be whitelisted. Together these meant dynamic_code_compile_validate_enable=true rejected every Java doc unconditionally, benign or not -- invisible to the existing test suite because no scenario combined a genuinely benign Java body with strict validation turned on. Fix getDynamicCodeDependentMethods to recurse through any same-class self-call rather than treat it as a leaf dependency, and make CompiledObjects.validateDependency() a no-op for the Java language (the real validation already happened inside createJavaHttp4sEndpoint).
…ourcedoc-java-support
…ourcedoc-java-support # Conflicts: # obp-api/src/main/scala/code/api/util/DynamicUtil.scala
…scan The Supplier.get() bridge-method exemption added for Java dynamic resource docs only unrolled one level: it appended a same-class method's own callees as raw dependency tuples instead of recursing into them. A Java method_body that factors logic into its own private helper methods (fully ordinary Java) reintroduces the exact same false rejection one level deeper -- the un-recursed helper's callees get flagged as calls to the compiled class's own randomly-generated, unwhitelistable name. Replace the one-level unroll with a proper recursive expansion that follows same-class calls to arbitrary depth until a genuinely foreign dependency is reached, with a visited-set cycle guard: direct or mutual recursion between same-class private methods (e.g. a factorial/fibonacci helper) is ordinary Java and must not recurse forever, and a cycle correctly contributes nothing further rather than falling back to a leaf that gets rejected the same way. Applies uniformly to both the new Java same-class case and the pre-existing Scala nested-closure case, which had the identical one-level limitation.
POST /obp/v6.0.0/management/dynamic-resource-docs/validate re-implemented the request_verb and example_request_body preconditions the v4 create endpoint checks, but not the programming_lang check added alongside Java support. CompiledObjects falls through to the Scala compile path for any value it doesn't recognise as "java"/"Java", so validate would report valid=true for a body that happens to compile as Scala under a bogus or misspelled programming_lang -- a verdict that contradicts what the create endpoint would actually do (400 DynamicCodeLangNotSupport), defeating the point of a dry-run validation endpoint. Add the same language check, mirroring Http4s400's validateDynamicResourceDocBody. No prior test exercised this endpoint at all; added coverage for the rejection and for both supported languages.
resourceDocs' per-row catch swallows any exception from toResourceDoc and logs it as "likely stored under the deprecated Lift contract -- re-author the body against the new native contract", regardless of cause. That is accurate for a genuine compile failure, but CompiledObjects.validateDependency now runs fresh on every construction (not just at create/update time), so a previously-registered doc can start throwing JsonResponseException here too -- purely because dynamic_code_compile_validate_dependencies was tightened after it was registered. The endpoint silently drops from the listing either way, but the log sends whoever reads it toward rewriting a body that was never the problem. Catch JsonResponseException separately and report the actual whitelist rejection reason (via the same JsonResponseExtractor pattern Http4s600's validate endpoint already uses for this exception shape), falling back to the existing generic message for every other cause. No dedicated test: this only changes log wording for a rejection path already covered end-to-end by DynamicResourceDocJavaSecurityValidationTest; asserting on log content here would need capture infrastructure this codebase doesn't otherwise use for a diagnostics-only change.
allowedCompilationMethods was changed from val to def so it observes a live props override (needed for setPropsValues-driven tests), but validateDependency's collect guard reads it twice per dependency tuple -- Map.get and a separate .exists scan. Each read re-derives the whitelist from scratch: a prop read, a regex-based rewrite of the whitelist source into compilable Scala, and a ConcurrentHashMap lookup keyed on that ~1.5KB string. For N dependency tuples that is up to 2N re-derivations where one would do. Bind it to a val at the top of the method instead. Still observes props changes between calls (each validateDependency invocation re-binds), just not multiple times within the same call.
createJavaHttp4sEndpoint reflectively reads the compiled class's bytes out of MemoryClassLoader's private mapClassBytes map to hand them to Javassist for dependency validation. The lookup result was passed straight to ByteArrayClassPath unchecked: if a future java-scriptengine version keys that map differently (internal name vs binary name, or drops the field entirely), get() returns null, and the failure only surfaces later as an opaque NPE inside Javassist -- nothing at the actual point of breakage indicates the cause was this reflective coupling to an internal field. Check for null and throw immediately with a message that names the field and the class it was looked up for. The throw happens inside the same Box tryo the rest of the compile step already runs in, so it is reported the same way any other compile failure is -- no behavior change on the success path. Also documents (no functional change) the pre-existing, unbounded-by-design memoClassPool/dynamicCompileResult caching tradeoff that the Java compile path now shares: each distinct compiled method_body retains its ClassLoader and ClassPool for the life of the process. This is consistent with the existing Scala compile cache's design (dynamic resource doc creation is gated behind operator-only roles, not open to arbitrary callers) rather than a new gap introduced here.
…e Docs Merge hongwei1's feature/dynamicresourcedoc-java-support onto develop. Conflict resolution against the 2026-09-23 dynamic-code changes: - DynamicUtil.Validation keeps the renamed props (dynamic_code_allowed_obp_methods, dynamic_code_obp_calls_are_restricted) read through legacyProp, and takes the branch's def-not-val reading so a props change applies without a restart. The legacy-name warning is now issued once per name, since the props are read on every validation. - The dependency allowlist check passes force = true, so it no longer silently depends on show_used_connector_methods. - The branch's dynamic_code_sandbox_permissions block is dropped; the sandbox was removed (SecurityManager is gone on JDK 24+). - CompiledObjects keeps its companion (wrapMethodBody, compileProblems) and gains programmingLang. - Http4s400 keeps both the field-length check and the programming_lang check. - Old prop names in the branch's comments and tests are renamed. - classOf[Sandbox] removed from the default allowlist in sample.props.template (and the test copy of it): the class no longer exists, so the allowlist failed to compile. - Frozen snapshot regenerated: adds the optional JsonDynamicResourceDoc.programmingLang field.
…mming_lang on v7.0.0 dry-run compile Code hashes cover the programming language as well as the method body. The maker/checker guard recomputes the hash from the row instead of trusting MethodBodyHash. Existing rows are rehashed once at boot, carrying over only approvals still valid for the current body. Approving a queued Java Dynamic Resource Doc now compiles it as Java. POST /management/dynamic-resource-docs/compile takes programming_lang and checks Java bodies without loading them. The Scala dry run checks code inside a method, as the real compile does, so a body using return is no longer rejected.
|
simonredfern
changed the title
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
2 participants
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.
No description provided.