This PR adds models Java client APIs for CouchBase and adds tests for 2 queries #21082
+726
−49
Conversation
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
mbaluda
changed the title
mbaluda
changed the title
Contributor
Click to show differences in coveragejavaGenerated file changes for java
- Others,"``actions.osgi``, ``antlr``, ``ch.ethz.ssh2``, ``cn.hutool.core.codec``, ``com.alibaba.com.caucho.hessian.io``, ``com.alibaba.druid.sql``, ``com.alibaba.fastjson2``, ``com.amazonaws.auth``, ``com.auth0.jwt.algorithms``, ``com.azure.identity``, ``com.caucho.burlap.io``, ``com.caucho.hessian.io``, ``com.cedarsoftware.util.io``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.esotericsoftware.yamlbeans``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.google.gson``, ``com.hubspot.jinjava``, ``com.jcraft.jsch``, ``com.microsoft.sqlserver.jdbc``, ``com.mitchellbosecke.pebble``, ``com.mongodb``, ``com.opensymphony.xwork2``, ``com.rabbitmq.client``, ``com.sshtools.j2ssh.authentication``, ``com.sun.crypto.provider``, ``com.sun.jndi.ldap``, ``com.sun.net.httpserver``, ``com.sun.net.ssl``, ``com.sun.rowset``, ``com.sun.security.auth.module``, ``com.sun.security.ntlm``, ``com.sun.security.sasl.digest``, ``com.thoughtworks.xstream``, ``com.trilead.ssh2``, ``com.unboundid.ldap.sdk``, ``com.zaxxer.hikari``, ``flexjson``, ``freemarker.cache``, ``freemarker.template``, ``groovy.lang``, ``groovy.text``, ``groovy.util``, ``hudson``, ``io.jsonwebtoken``, ``io.netty.bootstrap``, ``io.netty.buffer``, ``io.netty.channel``, ``io.netty.handler.codec``, ``io.netty.handler.ssl``, ``io.netty.handler.stream``, ``io.netty.resolver``, ``io.netty.util``, ``io.undertow.server.handlers.resource``, ``javafx.scene.web``, ``jenkins``, ``jodd.json``, ``liquibase.database.jvm``, ``liquibase.statement.core``, ``net.lingala.zip4j``, ``net.schmizz.sshj``, ``net.sf.json``, ``net.sf.saxon.s9api``, ``ognl``, ``okhttp3``, ``org.acegisecurity``, ``org.antlr.runtime``, ``org.apache.commons.codec``, ``org.apache.commons.compress.archivers.tar``, ``org.apache.commons.exec``, ``org.apache.commons.fileupload``, ``org.apache.commons.httpclient.util``, ``org.apache.commons.jelly``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.lang``, ``org.apache.commons.logging``, ``org.apache.commons.net``, ``org.apache.commons.ognl``, ``org.apache.cxf.catalog``, ``org.apache.cxf.common.classloader``, ``org.apache.cxf.common.jaxb``, ``org.apache.cxf.common.logging``, ``org.apache.cxf.configuration.jsse``, ``org.apache.cxf.helpers``, ``org.apache.cxf.resource``, ``org.apache.cxf.staxutils``, ``org.apache.cxf.tools.corba.utils``, ``org.apache.cxf.tools.util``, ``org.apache.cxf.transform``, ``org.apache.directory.ldap.client.api``, ``org.apache.hadoop.fs``, ``org.apache.hadoop.hive.metastore``, ``org.apache.hadoop.hive.ql.exec``, ``org.apache.hadoop.hive.ql.metadata``, ``org.apache.hc.client5.http.async.methods``, ``org.apache.hc.client5.http.classic.methods``, ``org.apache.hc.client5.http.fluent``, ``org.apache.hive.hcatalog.templeton``, ``org.apache.ibatis.jdbc``, ``org.apache.ibatis.mapping``, ``org.apache.log4j``, ``org.apache.shiro.authc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.apache.shiro.mgt``, ``org.apache.sshd.client.session``, ``org.apache.struts.beanvalidation.validation.interceptor``, ``org.apache.struts2``, ``org.apache.tools.ant``, ``org.apache.tools.zip``, ``org.apache.velocity.app``, ``org.apache.velocity.runtime``, ``org.codehaus.cargo.container.installer``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.eclipse.jetty.client``, ``org.exolab.castor.xml``, ``org.fusesource.leveldbjni``, ``org.geogebra.web.full.main``, ``org.gradle.api.file``, ``org.hibernate``, ``org.ho.yaml``, ``org.influxdb``, ``org.jabsorb``, ``org.jboss.vfs``, ``org.jdbi.v3.core``, ``org.jenkins.ui.icon``, ``org.jenkins.ui.symbol``, ``org.jooq``, ``org.keycloak.models.map.storage``, ``org.kohsuke.stapler``, ``org.lastaflute.web``, ``org.mvel2``, ``org.openjdk.jmh.runner.options``, ``org.owasp.esapi``, ``org.pac4j.jwt.config.encryption``, ``org.pac4j.jwt.config.signature``, ``org.scijava.log``, ``org.slf4j``, ``org.thymeleaf``, ``org.xml.sax``, ``org.xmlpull.v1``, ``org.yaml.snakeyaml``, ``play.libs.ws``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``, ``retrofit2``, ``software.amazon.awssdk.transfer.s3.model``, ``sun.jvmstat.perfdata.monitor.protocol.local``, ``sun.jvmstat.perfdata.monitor.protocol.rmi``, ``sun.misc``, ``sun.net.ftp``, ``sun.net.www.protocol.http``, ``sun.security.acl``, ``sun.security.jgss.krb5``, ``sun.security.krb5``, ``sun.security.pkcs``, ``sun.security.pkcs11``, ``sun.security.provider``, ``sun.security.ssl``, ``sun.security.x509``, ``sun.tools.jconsole``",144,10529,927,140,6,22,18,,208
+ Others,"``actions.osgi``, ``antlr``, ``ch.ethz.ssh2``, ``cn.hutool.core.codec``, ``com.alibaba.com.caucho.hessian.io``, ``com.alibaba.druid.sql``, ``com.alibaba.fastjson2``, ``com.amazonaws.auth``, ``com.auth0.jwt.algorithms``, ``com.azure.identity``, ``com.caucho.burlap.io``, ``com.caucho.hessian.io``, ``com.cedarsoftware.util.io``, ``com.couchbase.client.core.env``, ``com.couchbase.client.java``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.esotericsoftware.yamlbeans``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.google.gson``, ``com.hubspot.jinjava``, ``com.jcraft.jsch``, ``com.microsoft.sqlserver.jdbc``, ``com.mitchellbosecke.pebble``, ``com.mongodb``, ``com.opensymphony.xwork2``, ``com.rabbitmq.client``, ``com.sshtools.j2ssh.authentication``, ``com.sun.crypto.provider``, ``com.sun.jndi.ldap``, ``com.sun.net.httpserver``, ``com.sun.net.ssl``, ``com.sun.rowset``, ``com.sun.security.auth.module``, ``com.sun.security.ntlm``, ``com.sun.security.sasl.digest``, ``com.thoughtworks.xstream``, ``com.trilead.ssh2``, ``com.unboundid.ldap.sdk``, ``com.zaxxer.hikari``, ``flexjson``, ``freemarker.cache``, ``freemarker.template``, ``groovy.lang``, ``groovy.text``, ``groovy.util``, ``hudson``, ``io.jsonwebtoken``, ``io.netty.bootstrap``, ``io.netty.buffer``, ``io.netty.channel``, ``io.netty.handler.codec``, ``io.netty.handler.ssl``, ``io.netty.handler.stream``, ``io.netty.resolver``, ``io.netty.util``, ``io.undertow.server.handlers.resource``, ``javafx.scene.web``, ``jenkins``, ``jodd.json``, ``liquibase.database.jvm``, ``liquibase.statement.core``, ``net.lingala.zip4j``, ``net.schmizz.sshj``, ``net.sf.json``, ``net.sf.saxon.s9api``, ``ognl``, ``okhttp3``, ``org.acegisecurity``, ``org.antlr.runtime``, ``org.apache.commons.codec``, ``org.apache.commons.compress.archivers.tar``, ``org.apache.commons.exec``, ``org.apache.commons.fileupload``, ``org.apache.commons.httpclient.util``, ``org.apache.commons.jelly``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.lang``, ``org.apache.commons.logging``, ``org.apache.commons.net``, ``org.apache.commons.ognl``, ``org.apache.cxf.catalog``, ``org.apache.cxf.common.classloader``, ``org.apache.cxf.common.jaxb``, ``org.apache.cxf.common.logging``, ``org.apache.cxf.configuration.jsse``, ``org.apache.cxf.helpers``, ``org.apache.cxf.resource``, ``org.apache.cxf.staxutils``, ``org.apache.cxf.tools.corba.utils``, ``org.apache.cxf.tools.util``, ``org.apache.cxf.transform``, ``org.apache.directory.ldap.client.api``, ``org.apache.hadoop.fs``, ``org.apache.hadoop.hive.metastore``, ``org.apache.hadoop.hive.ql.exec``, ``org.apache.hadoop.hive.ql.metadata``, ``org.apache.hc.client5.http.async.methods``, ``org.apache.hc.client5.http.classic.methods``, ``org.apache.hc.client5.http.fluent``, ``org.apache.hive.hcatalog.templeton``, ``org.apache.ibatis.jdbc``, ``org.apache.ibatis.mapping``, ``org.apache.log4j``, ``org.apache.shiro.authc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.apache.shiro.mgt``, ``org.apache.sshd.client.session``, ``org.apache.struts.beanvalidation.validation.interceptor``, ``org.apache.struts2``, ``org.apache.tools.ant``, ``org.apache.tools.zip``, ``org.apache.velocity.app``, ``org.apache.velocity.runtime``, ``org.codehaus.cargo.container.installer``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.eclipse.jetty.client``, ``org.exolab.castor.xml``, ``org.fusesource.leveldbjni``, ``org.geogebra.web.full.main``, ``org.gradle.api.file``, ``org.hibernate``, ``org.ho.yaml``, ``org.influxdb``, ``org.jabsorb``, ``org.jboss.vfs``, ``org.jdbi.v3.core``, ``org.jenkins.ui.icon``, ``org.jenkins.ui.symbol``, ``org.jooq``, ``org.keycloak.models.map.storage``, ``org.kohsuke.stapler``, ``org.lastaflute.web``, ``org.mvel2``, ``org.openjdk.jmh.runner.options``, ``org.owasp.esapi``, ``org.pac4j.jwt.config.encryption``, ``org.pac4j.jwt.config.signature``, ``org.scijava.log``, ``org.slf4j``, ``org.thymeleaf``, ``org.xml.sax``, ``org.xmlpull.v1``, ``org.yaml.snakeyaml``, ``play.libs.ws``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``, ``retrofit2``, ``software.amazon.awssdk.transfer.s3.model``, ``sun.jvmstat.perfdata.monitor.protocol.local``, ``sun.jvmstat.perfdata.monitor.protocol.rmi``, ``sun.misc``, ``sun.net.ftp``, ``sun.net.www.protocol.http``, ``sun.security.acl``, ``sun.security.jgss.krb5``, ``sun.security.krb5``, ``sun.security.pkcs``, ``sun.security.pkcs11``, ``sun.security.provider``, ``sun.security.ssl``, ``sun.security.x509``, ``sun.tools.jconsole``",144,10547,950,140,6,34,18,,208
- Totals,,363,26371,2656,404,16,128,33,1,409
+ Totals,,363,26389,2679,404,16,140,33,1,409
+ com.couchbase.client.core.env,7,,,,,1,4,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ com.couchbase.client.java,16,,18,,,,2,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,12,,,,,,,,,,,,,,18, |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds security analysis support for the Couchbase Java client library by introducing sink and summary models that enable detection of SQL Injection and Hardcoded Credentials vulnerabilities. The PR includes comprehensive test stubs, test cases with expected outputs, and model definitions for the com.couchbase.client package.
Key Changes:
- Added sink models for SQL injection detection in query methods (
Cluster.query,Collection.upsert,Collection.replace) - Added sink models for hardcoded credentials detection in authentication methods
- Added summary models for taint propagation through
JsonObjectmethods
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| java/ql/test/stubs/couchbaseClient/* | Test stub implementations for Couchbase client classes (17 files) |
| java/ql/test/query-tests/security/CWE-798/semmle/tests/options | Updated classpath to include couchbaseClient stubs |
| java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCouchBaseCredentials.java | Test case for hardcoded credentials detection with expected annotations |
| java/ql/test/query-tests/security/CWE-089/semmle/examples/options | Updated classpath to include couchbaseClient stubs |
| java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.expected | Updated expected test results with new Couchbase SQL injection findings |
| java/ql/test/query-tests/security/CWE-089/semmle/examples/CouchBase.java | Test case demonstrating SQL injection vulnerabilities in Couchbase usage |
| java/ql/src/change-notes/2025-23-23-couchbase-sinks.md | Release note documenting the new Couchbase security models |
| java/ql/lib/ext/com.couchbase.client.java.model.yml | Sink and summary model definitions for main Couchbase client classes |
| java/ql/lib/ext/com.couchbase.client.core.env.model.yml | Sink model definitions for Couchbase authentication classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
owen-mc
requested changes
Jan 10, 2026
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKeyStore", "(KeyStore,String)", "", "Argument[1]", "credentials-password", "manual"] | ||
| - ["com.couchbase.client.core.env", "PasswordAuthenticator$Builder", true, "username", "(String)", "", "Argument[0]", "credentials-username", "manual"] | ||
| - ["com.couchbase.client.core.env", "PasswordAuthenticator$Builder", true, "username", "(Supplier<String>)", "", "Argument[0]", "credentials-username", "manual"] | ||
| - ["com.couchbase.client.core.env", "PasswordAuthenticator$Builder", true, "password", "(String)", "", "Argument[0]", "credentials-password", "manual"] |
Contributor
There was a problem hiding this comment.
What about the other overload of password? And also PasswordAuthenticator#builder, PasswordAuthenticator#create and PasswordAuthenticatore#ldapCompatible? (I'm looking at the docs here.)
| data: | ||
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKey", "(PrivateKey,String,List)", "", "Argument[0]", "credentials-key", "manual"] | ||
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKey", "(PrivateKey,String,List)", "", "Argument[1]", "credentials-password", "manual"] | ||
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKeyStore", "(Path,String,Optional<String>)", "", "Argument[1]", "credentials-password", "manual"] |
Contributor
There was a problem hiding this comment.
Suggested change
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKeyStore", "(Path,String,Optional<String>)", "", "Argument[1]", "credentials-password", "manual"] | |
| - ["com.couchbase.client.core.env", "CertificateAuthenticator", true, "fromKeyStore", "(Path,String,Optional)", "", "Argument[1]", "credentials-password", "manual"] |
We just use the erased type. I don't think the model will work as it stands.
Please remove all types in angle brackets in other models as well.
Comment on lines
+16
to
+17
| - ["com.couchbase.client.java", "Cluster", true, "searchQuery", "(String,SearchQuery)", "", "Argument[1]", "sql-injection", "manual"] | ||
| - ["com.couchbase.client.java", "Cluster", true, "searchQuery", "(String,SearchQuery,SearchOptions)", "", "Argument[1]", "sql-injection", "manual"] |
Contributor
There was a problem hiding this comment.
While these models are correct, they currently don't have any effect, because we don't have any models for taint to flow into a SearchQuery. Ideally we would also model some methods for creating (subclasses of) SearchQuery. I won't block this PR on this, however.
Comment on lines
+10
to
+11
| - ["com.couchbase.client.java", "Cluster", true, "query", "(String)", "", "Argument[0]", "sql-injection", "manual"] | ||
| - ["com.couchbase.client.java", "Cluster", true, "query", "(String,QueryOptions)", "", "Argument[0]", "sql-injection", "manual"] |
Contributor
There was a problem hiding this comment.
These should be lower down, to keep an alphabetical ordering, so it's easier to find them in future. Actually I think we should keep models with the same kind (e.g. sql-injection) together, with a comment between the different blocks.
Comment on lines
+18
to
+21
| - ["com.couchbase.client.java", "Collection", true, "upsert", "(String,Object)", "", "Argument[1]", "sql-injection", "manual"] | ||
| - ["com.couchbase.client.java", "Collection", true, "upsert", "(String,Object,UpsertOptions)", "", "Argument[1]", "sql-injection", "manual"] | ||
| - ["com.couchbase.client.java", "Collection", true, "replace", "(String,Object)", "", "Argument[1]", "sql-injection", "manual"] | ||
| - ["com.couchbase.client.java", "Collection", true, "replace", "(String,Object,ReplaceOptions)", "", "Argument[1]", "sql-injection", "manual"] |
Contributor
There was a problem hiding this comment.
These methods don't execute queries, so they shouldn't be sql-injection sinks. They could possibly be sinks with a different sink kind, but it isn't immediately obvious to me what that would be.
Suggested change
| - ["com.couchbase.client.java", "Collection", true, "upsert", "(String,Object)", "", "Argument[1]", "sql-injection", "manual"] | |
| - ["com.couchbase.client.java", "Collection", true, "upsert", "(String,Object,UpsertOptions)", "", "Argument[1]", "sql-injection", "manual"] | |
| - ["com.couchbase.client.java", "Collection", true, "replace", "(String,Object)", "", "Argument[1]", "sql-injection", "manual"] | |
| - ["com.couchbase.client.java", "Collection", true, "replace", "(String,Object,ReplaceOptions)", "", "Argument[1]", "sql-injection", "manual"] |
Comment on lines
+27
to
+28
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] | ||
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[1]", "ReturnValue", "taint", "manual"] |
Contributor
There was a problem hiding this comment.
I'm not sure why you're modelling flow through JsonObject. Do any of the methods modelled above (or already modelled) use JsonObject? I don't remember seeing any. Or is it that JsonObjects are often used but aren't mandated?
The models are fine, so there is no need to remove them. I only bring it up in case it makes it clear that there's a gap in the modelling somewhere.
JsonObject is basically a Map, so we could use the MapKey and MapValue access paths so that we can distinguish them.
Suggested change
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] | |
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[1]", "ReturnValue", "taint", "manual"] | |
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[0]", "ReturnValue.MapKey", "taint", "manual"] | |
| - ["com.couchbase.client.java.json", "JsonObject", true, "put", "(String,Object)", "", "Argument[1]", "ReturnValue.MapValue", "taint", "manual"] |
This should also be done for all the below models... except I'm actually going to suggest that your replace all of the models for put with these two:
- ["com.couchbase.client.java.json", "JsonObject", true, "put", "", "", "Argument[0]", "ReturnValue.MapKey", "taint", "manual"]
- ["com.couchbase.client.java.json", "JsonObject", true, "put", "", "", "Argument[1]", "ReturnValue.MapValue", "taint", "manual"]
(When no signature is specified, the model applies to all methods with that name.)
Contributor
There was a problem hiding this comment.
I would love it if these tests could exercise more (or most? or even all?) of the models you've added. That is the only way of checking that the models work - and in this case it would have caught the issues with angle brackets.
(I would also love it if the tests were converted to inline expectations. But there is no need for you to do that.)
Contributor
There was a problem hiding this comment.
Again, it would be better if there were more comprehensive tests.
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.
This pull request adds models for the
com.couchbaseJava client, enabling detection of SQL Injection and Hardcoded Credentials vulnerabilities.It also updates test cases and documentation to reflect these enhancements.