Skip to content

Java rest api - #1063

Open
achmelo wants to merge 38 commits into
staging/java-rest-apifrom
java-rest-api
Open

Java rest api#1063
achmelo wants to merge 38 commits into
staging/java-rest-apifrom
java-rest-api

Conversation

@achmelo

@achmelo achmelo commented Jul 7, 2026

Copy link
Copy Markdown
Member

What It Does
Spring REST API for zowex that integrates with API ML.

How to Test

Review Checklist
I certify that I have:

Additional Comments

achmelo and others added 27 commits April 30, 2026 14:22
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: Richard Salac <richard.salac@broadcom.com>
# Conflicts:
#	native/java/app/src/main/java/org/zowe/zowex/ffm/ZjbBindings.java
#	native/java/app/src/main/java/org/zowe/zowex/ffm/ZusfBindings.java
Signed-off-by: ac892247 <a.chmelo@gmail.com>
…ndler

Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
Signed-off-by: ac892247 <a.chmelo@gmail.com>
traeok
traeok previously requested changes Jul 8, 2026

@traeok traeok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for all your work on this @achmelo! This looks great and I'm looking forward to seeing this in the project. I left some suggestions and requested changes regarding the non-freed password block.

Comment thread native/java/secur/secur.c
{
int rc = EINVAL;
char *platformUser = jstring_to_ebcdic(env, user);
char *platformPassword = jstring_to_ebcdic(env, password);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This variable is allocated but only platformUser and platformApplId are passed to free_if_not_null. Does this mean that every login leaks an un-scrubbed copy of the user's plaintext password? If so, could we scrub it and then free it?

For example:

if (platformPassword != NULL) {
    size_t len = strlen(platformPassword);
    memset(platformPassword, 0, len);   // scrub before free
    free(platformPassword);
}

Comment thread native/java/secur/secur.c
#define THLIAPPLIDLEN 0x052
#define THLIAPPLID 0x070

int lastErrno2 = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a process-level global and not thread-local. Couldn't two concurrent requests race on this variable? See comment below about Java_org_zowe_zowex_zos_security_jni_Secur_createSecurityEnvironment

Comment thread native/java/secur/secur.c
}
free_if_not_null(platformUser);
free_if_not_null(platformApplId);
return rc;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could pack rc and errno2 into a single jlong to avoid the process-level global. We could defer this to a future PR if this requires out-of-scope changes.

Comment thread native/java/secur/secur.c
memset(thliApplid, ' ', 8);
origApplid[8] = 0;
memcpy(origApplid, thliApplid, 8);
memcpy(thliApplid, applid, applidLength);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we check the length against the SAF THLI field's actual size? Otherwise, copying data w/ a length greater than the THLI field may corrupt adjacent memory or crash the JVM.

For example:

char *applid = jstring_to_ebcdic(env, jApplid);
if (applid == NULL) {
    return -1;  // conversion failed
}
const int applidLength = strlen(applid);
if (applidLength > 8) {
    free_if_not_null(applid);
    return -1;  // reject anything that won't fit the THLI field
}

@Tag(name = "Security")
@RestController
@RequestMapping("/api/v1/securityTest")
public class SecurityContextController {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correct me if I'm missing something, but is it true that any authenticated caller could use this controller to query whether any user has a given SAF access level to a specific resource?

For context, the class comment says "Troubleshooting only" , but there's no @Profile or admin-only guard, suggesting it could be reached by any authenticated caller in production.

}
}
// Free memory
ZdsCApi.zds_c_free_list_response(responsePtr);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If any of the readString calls fail in the above loop, the native block is never freed and it jumps straight to the catch on lines 77-80. Should we add a small helper to guarantee cleanup regardless of the outcome?

This same pattern occurs in other functions within this file, hence the suggestion for the helper.

return null;
}
// Reinterpret the segment to maximum size so getString can find the null terminator
return segment.reinterpret(Long.MAX_VALUE).getString(0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not opposed to using Long.MAX_VALUE, but is there a smaller and more reasonable upper bound for string read?

The Long.MAX_VALUE may allow for an unbounded memory scan, or possible segmentation fault into unmapped pages of memory.

@zFernand0
zFernand0 self-requested a review July 13, 2026 15:02
@achmelo

achmelo commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Thanks for all your work on this @achmelo! This looks great and I'm looking forward to seeing this in the project. I left some suggestions and requested changes regarding the non-freed password block.

Thanks @traeok for the review. I tried to address all your comments, could you please take a look?

@traeok
traeok self-requested a review July 15, 2026 13:13
@traeok
traeok dismissed their stale review July 16, 2026 19:12

Requested changes were implemented, taking another look

@traeok

traeok commented Jul 20, 2026

Copy link
Copy Markdown
Member

I see two critical CodeQL issues flagged around use of strcpy and possible unbounded writes.
Can you please address these? The other warnings are lower in severity, but I think those two are valid.

@CBforZ

CBforZ commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Could we add a readme for building & running the Java server please? I assume it involves the gradle tasks, but would be nice to have a guide checked in for the basics

@traeok traeok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Plan to test extensively & will likely approve once the critical CodeQL issues are addressed.

achmelo and others added 3 commits July 24, 2026 11:34
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: achmelo <37397715+achmelo@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: achmelo <37397715+achmelo@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: achmelo <37397715+achmelo@users.noreply.github.com>
@JTonda
JTonda requested a review from traeok July 27, 2026 15:02

@traeok traeok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for addressing the CodeQL issues.
Echoing @CBforZ suggestion: can you add a Markdown file with information on how to set up and use the Java REST API?

Also, some steps in How to Test would be greatly appreciated - sorry, I'm a bit of a newb when it comes to Java + z/OS - but it would also benefit other folks that are reviewing 😋

@traeok

traeok commented Aug 11, 2026

Copy link
Copy Markdown
Member

Hi @achmelo, I was wondering if you had a chance to review/address the latest round of feedback? Primarily around the readme + documentation for how to get the server set up 🙂 thanks!

Signed-off-by: ac892247 <a.chmelo@gmail.com>
@achmelo

achmelo commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Hi @achmelo, I was wondering if you had a chance to review/address the latest round of feedback? Primarily around the readme + documentation for how to get the server set up 🙂 thanks!

sorry for late response, I was occupied with another work. I have added readme with details that I remember. Hope I didn't miss anything. Please, let me know if something doesn't work.

@traeok

traeok commented Aug 12, 2026

Copy link
Copy Markdown
Member

Hi @achmelo, I was wondering if you had a chance to review/address the latest round of feedback? Primarily around the readme + documentation for how to get the server set up 🙂 thanks!

sorry for late response, I was occupied with another work. I have added readme with details that I remember. Hope I didn't miss anything. Please, let me know if something doesn't work.

no worries @achmelo , thanks for the updates! I'll take a look

@JTonda
JTonda requested a review from traeok August 12, 2026 15:09
@CBforZ
CBforZ self-requested a review August 12, 2026 15:10
Comment thread native/java/bindings/zjb_c_api.cpp Outdated
std::string jobid_str = jobid ? jobid : "";
a2e_inplace(jobid_str);

int rc = zjb_read_jobs_output_by_key(&zjb, jobid_str, key, out_response);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was renamed to zjb_read_job_content_by_key , needs that update to compile after updating from the main branch

Signed-off-by: CBforZ <chris.boehm@broadcom.com>
@CBforZ
CBforZ changed the base branch from main to staging/java-rest-api September 1, 2026 14:16
Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@traeok traeok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @achmelo! Changes LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Review/QA

Development

Successfully merging this pull request may close these issues.

7 participants