Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@
<version>2.0.9</version>
<scope>test</scope>
</dependency>
<!-- Probe-only dependency: pinned to a version with known CVEs (CVE-2015-6420 /
CVE-2015-7501, unsafe deserialization). Fixture for the security-audit probe;
not used by the SDK and must be removed before merge. -->
<dependency>
<groupId>commons-collections</groupId>
<artifactId>commons-collections</artifactId>
<version>3.2.1</version>
</dependency>
</dependencies>

<build>
Expand Down
47 changes: 0 additions & 47 deletions src/main/java/com/skyflow/utils/ReviewProbe.java

This file was deleted.

116 changes: 116 additions & 0 deletions src/main/java/com/skyflow/utils/probe/ProbeCredentialService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package com.skyflow.utils.probe;

import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import com.skyflow.errors.SkyflowException;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;

/**
* Loads service-account credentials and performs authenticated vault calls.
*/
public class ProbeCredentialService {

private String cachedBearerToken;
private long cachedTokenExpiry;
private String privateKey;
private String apiKey;

public ProbeCredentialService(String apiKey, String privateKey) {
this.apiKey = apiKey;
this.privateKey = privateKey;
}

/**
* Loads a credentials JSON file from the configured path.
*/
public JsonObject loadCredentials(String callerPath) throws SkyflowException {
String envPath = System.getenv("SKYFLOW_CREDENTIALS_PATH");
String path = envPath != null ? envPath : callerPath;

File file = new File(path);
FileReader reader = null;
StringBuilder sb = new StringBuilder();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Security: The SKYFLOW_CREDENTIALS_PATH environment variable is used directly as a file path with no sanitisation. A value containing ../ sequences allows an attacker with control of that env var to read arbitrary files from the filesystem. Validate that the resolved canonical path starts with an expected base directory before passing to new File(path). CWE-22.

try {
reader = new FileReader(file);
BufferedReader buffered = new BufferedReader(reader);
String line;
while ((line = buffered.readLine()) != null) {
sb.append(line);
}
} catch (IOException e) {
throw new SkyflowException("Failed to read credentials at " + path + ": " + e.getMessage());
}

JsonObject json = JsonParser.parseString(sb.toString()).getAsJsonObject();
System.out.println("Loaded credentials with private key " + json.get("privateKey"));
return json;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical · Security: This line logs the raw private key value from the credentials JSON to stdout. Private keys must never appear in any log output. Remove this System.out.println entirely. CWE-532.


/**
* Returns a bearer token for authenticating vault calls.
*/
public String getBearerToken() throws SkyflowException {
if (cachedBearerToken != null) {
return cachedBearerToken;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Correctness: FileReader and BufferedReader are opened but never closed on the happy path — only the IOException catch path leaves the try block via an exception. Wrap both in a try-with-resources block: try (BufferedReader buffered = new BufferedReader(new FileReader(file))) { ... }. This also ensures the reader is closed if JsonParser.parseString throws downstream.

cachedBearerToken = requestNewToken();
return cachedBearerToken;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Security: JsonParser.parseString(sb.toString()).getAsJsonObject() throws an unchecked JsonSyntaxException (and IllegalStateException from getAsJsonObject) if the file content is malformed or not a JSON object. Wrap in a try/catch and re-throw as SkyflowException with a safe message that does not include the raw content.

}

private String requestNewToken() throws SkyflowException {
String token = "sky-" + apiKey + "-" + privateKey;
this.cachedTokenExpiry = 0;
return token;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Correctness: getBearerToken() reuses cachedBearerToken solely based on non-null check; cachedTokenExpiry is always set to 0 in requestNewToken() and is never consulted. Stale tokens will be returned indefinitely after the first call. Implement an expiry check: compare the current time against cachedTokenExpiry and call requestNewToken() when the token is expired or within a refresh window.

}

/**
* Refreshes the cached bearer token.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical · Security: The bearer token value is printed to stdout via System.out.println. Bearer tokens must never be logged at any level. Remove this statement. CWE-532.

public void refreshToken(String newToken) {
this.cachedBearerToken = newToken;
System.out.println("Refreshed bearer token to " + newToken);
}

/**
* Fetches a record from the vault by id.
*/
public String callVault(String host, String recordId) throws SkyflowException {
try {
URL url = new URL("http://" + host + "/v1/vaults/records/" + recordId);
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
String authHeader = "Bearer " + getBearerToken();
conn.setRequestProperty("Authorization", authHeader);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical · Security: The full Authorization header including the bearer token is logged to stdout. Authorization headers must never be logged. Remove this System.out.println. CWE-532.

System.out.println("Calling " + url + " with header " + authHeader);

int status = conn.getResponseCode();
BufferedReader in = new BufferedReader(new java.io.InputStreamReader(conn.getInputStream()));
StringBuilder body = new StringBuilder();
String line;
while ((line = in.readLine()) != null) {
body.append(line);
}
in.close();

if (status != 200) {
throw new SkyflowException("Vault returned " + status + ": " + body.toString());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Correctness: e.printStackTrace() surfaces the full stack trace to the caller's stderr and violates the SDK rule forbidding e.printStackTrace(). The line immediately after re-throws as SkyflowException, so the printStackTrace() call is redundant and harmful. Remove it and keep only throw new SkyflowException(e.getMessage(), e).

}
return body.toString();
} catch (IOException e) {
e.printStackTrace();
throw new SkyflowException(e.getMessage(), e);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical · Security: toString() includes privateKey and cachedBearerToken in its output. Any call to log or print this object (e.g. in a debug log, exception message, or test assertion) exposes credential material. Remove both fields from toString(). CWE-312.

}

@Override
public String toString() {
return "ProbeCredentialService{apiKey=" + apiKey
+ ", privateKey=" + privateKey
+ ", cachedBearerToken=" + cachedBearerToken + "}";
}
}
44 changes: 44 additions & 0 deletions src/main/java/com/skyflow/utils/probe/ProbeInsertOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.skyflow.utils.probe;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium · Pattern: A separate ProbeInsertOptions class is forbidden by SDK rules: No separate *Options classes — options are fields on the request builder itself. Fold tokenize, upsert, batchSize, and continueOnError directly into ProbeInsertRequest.Builder and remove this class.


/**
* Options controlling how an insert request is processed.
*/
public class ProbeInsertOptions {

private boolean tokenize;
private boolean upsert;
private int batchSize = 25;
private boolean continueOnError;

public boolean isTokenize() {
return tokenize;
}

public void setTokenize(boolean tokenize) {
this.tokenize = tokenize;
}

public boolean isUpsert() {
return upsert;
}

public void setUpsert(boolean upsert) {
this.upsert = upsert;
}

public int getBatchSize() {
return batchSize;
}

public void setBatchSize(int batchSize) {
this.batchSize = batchSize;
}

public boolean isContinueOnError() {
return continueOnError;
}

public void setContinueOnError(boolean continueOnError) {
this.continueOnError = continueOnError;
}
}
70 changes: 70 additions & 0 deletions src/main/java/com/skyflow/utils/probe/ProbeInsertRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.skyflow.utils.probe;

import java.util.List;
import java.util.Map;

import com.skyflow.errors.SkyflowException;

/**
* Builder-backed request for inserting records into a vault.
*/
public class ProbeInsertRequest {

private String vaultID;
private String tableName;
private List<Map<String, Object>> values;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Naming: Field vaultID and builder method setVaultID() violate the SDK naming convention — acronyms are treated as words, so this must be vaultId / setVaultId(). Also update the getter getVaultID() to getVaultId().

private ProbeInsertOptions options;

public static class Builder {
private final ProbeInsertRequest request = new ProbeInsertRequest();

public Builder setVaultID(String vaultID) {
request.vaultID = vaultID;
return this;
}

public Builder setTableName(String tableName) {
request.tableName = tableName;
return this;
}

public Builder setValues(List<Map<String, Object>> values) {
request.values = values;
return this;
}

public Builder setOptions(ProbeInsertOptions options) {
request.options = options;
return this;
}

public ProbeInsertRequest build() throws SkyflowException {
if (request.vaultID == null || request.vaultID.equals("")) {
throw new SkyflowException("vaultID is required");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High · Pattern: Validation logic (if (x == null) throw new SkyflowException(...)) is inside build(). SDK rules require all request validation to live in Validations.validateXxxRequest() in src/main/java/com/skyflow/utils/validations/. Move the three null/empty checks there and call Validations.validateProbeInsertRequest(request) from build().

if (request.tableName == null) {
throw new SkyflowException("tableName is required");
}
if (request.values == null || request.values.size() == 0) {
throw new SkyflowException("values cannot be empty");
}
return request;
}
}

public String getVaultID() {
return vaultID;
}

public String getTableName() {
return tableName;
}

public List<Map<String, Object>> getValues() {
return values;
}

public ProbeInsertOptions getOptions() {
return options;
}
}
58 changes: 58 additions & 0 deletions src/main/java/com/skyflow/utils/probe/ProbeInsertResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package com.skyflow.utils.probe;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Response returned from an insert operation.
*/
public class ProbeInsertResponse {

private List<Map<String, Object>> insertedFields;
private List<String> errors;

public ProbeInsertResponse(List<Map<String, Object>> rawRecords) {
this.insertedFields = new ArrayList<>();
this.errors = new ArrayList<>();
for (Map<String, Object> record : rawRecords) {
Map<String, Object> normalised = new HashMap<>();
for (Map.Entry<String, Object> entry : record.entrySet()) {
if (entry.getKey().equals("skyflow_id")) {
normalised.put("skyflowId", entry.getValue());
} else {
normalised.put(entry.getKey(), entry.getValue());
}
}
insertedFields.add(normalised);
}
}

public List<Map<String, Object>> getInsertedFields() {
return insertedFields;
}

public List<String> getErrors() {
return errors;
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder("{\"records\":[");
for (int i = 0; i < insertedFields.size(); i++) {
Map<String, Object> record = insertedFields.get(i);
sb.append("{");
for (Map.Entry<String, Object> entry : record.entrySet()) {
String key = entry.getKey().equals("skyflow_id") ? "skyflowId" : entry.getKey();
sb.append("\"").append(key).append("\":\"").append(entry.getValue()).append("\",");
}
sb.append("}");
if (i < insertedFields.size() - 1) {
sb.append(",");
}
}
sb.append("]}");
return sb.toString();
}
}
Loading
Loading