Fix/uss chown

native/CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to the native code for "zowe-native-proto" are documented in
 
 Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how to structure this file.
 
+## Recent Changes
+
+- `c`: Fixed issue where `zowex uss chown` (and `zusf_chown_uss_file_or_dir`) silently succeeded with exit code `0` when a non-existent user or group was supplied. The command now validates `user:group` input and returns a non-zero exit code with a clear error message when invalid. [#565](https://github.com/zowe/zowe-native-proto/pull/565)
+
 ## `0.1.9`
 
 - `golang`: Fixed an issue where an empty response on the `HandleReadFileRequest` function would result in a panic. [#550](https://github.com/zowe/zowe-native-proto/pull/550)

native/c/test/zusf.test.cpp

@@ -28,6 +28,153 @@ using namespace ztst;
 
 void zusf_tests()
 {
+  describe("zusf_chown_uss_file_or_dir tests",
+          [&]() -> void
+          {
+            ZUSF zusf;
+            memset(&zusf, 0, sizeof(zusf));
+
+            const std::string tmp_base = "/tmp/zusf_chown_tests";
+            const std::string file_path = tmp_base + "/one.txt";
+            const std::string dir_path  = tmp_base + "/tree";
+            const std::string dir_a     = dir_path + "/subA";
+            const std::string dir_b     = dir_path + "/subA/subB";
+            const std::string f_top     = dir_path + "/file_top.txt";
+            const std::string f_mid     = dir_a    + "/file_mid.txt";
+            const std::string f_bot     = dir_b    + "/file_bottom.txt";
+
+            // Ensure clean slate
+            system(("rm -rf " + tmp_base).c_str());
+            mkdir(tmp_base.c_str(), 0755);
+
+            // Discover current primary group (gid and name)
+            gid_t primary_gid = getgid();
+            struct group *gr = getgrgid(primary_gid);
+            const char *primary_group_name = (gr && gr->gr_name) ? gr->gr_name : nullptr;
+
+            it("should fail when path does not exist",
+              [&]() -> void
+              {
+                std::string not_there = tmp_base + "/does_not_exist.txt";
+                int rc = zusf_chown_uss_file_or_dir(&zusf, not_there, ":somegroup", false);
+                Expect(rc).ToBe(RTNCD_FAILURE);
+                Expect(std::string(zusf.diag.e_msg)).ToContain("does not exist");
+              });
+
+            it("should fail for invalid user name",
+              [&]() -> void
+              {
+                // Create a real file to exercise the user validation path
+                {
+                  std::ofstream f(file_path); f << "x"; f.close();
+                }
+                int rc = zusf_chown_uss_file_or_dir(&zusf, file_path, "nosuchuser_xyz", false);
+                Expect(rc).ToBe(RTNCD_FAILURE);
+                Expect(std::string(zusf.diag.e_msg)).ToContain("invalid user 'nosuchuser_xyz'");
+                unlink(file_path.c_str());
+              });
+
+            it("should fail for invalid group name",
+              [&]() -> void
+              {
+                {
+                  std::ofstream f(file_path); f << "x"; f.close();
+                }
+                int rc = zusf_chown_uss_file_or_dir(&zusf, file_path, ":nosuchgroup_xyz", false);
+                Expect(rc).ToBe(RTNCD_FAILURE);
+                Expect(std::string(zusf.diag.e_msg)).ToContain("invalid group 'nosuchgroup_xyz'");
+                unlink(file_path.c_str());
+              });
+
+            it("should fail for no-op guard ':' (no user or group specified, empty)",
+              [&]() -> void
+              {
+                {
+                  std::ofstream f(file_path); f << "x"; f.close();
+                }
+                int rc = zusf_chown_uss_file_or_dir(&zusf, file_path, ":", false);
+                Expect(rc).ToBe(RTNCD_FAILURE);
+                Expect(std::string(zusf.diag.e_msg)).ToContain("neither user nor group specified");
+                unlink(file_path.c_str());
+              });
+
+            it("should fail on directory without --recursive",
+              [&]() -> void
+              {
+                mkdir(dir_path.c_str(), 0755);
+                int rc = zusf_chown_uss_file_or_dir(&zusf, dir_path, ":somegroup", false);
+                Expect(rc).ToBe(RTNCD_FAILURE);
+                Expect(std::string(zusf.diag.e_msg)).ToContain("is a folder and recursive is false");
+                rmdir(dir_path.c_str());
+              });
+
+            it("should chown group-only on a single file when caller owns it",
+              [&]() -> void
+              {
+                if (!primary_group_name) {
+                  // If we cannot resolve a group name, skip meaningfully
+                  std::cout << "[SKIP] primary group name not available\n";
+                  return;
+                }
+
+                { std::ofstream f(file_path); f << "hello"; }
+
+                std::string owner_str = std::string(":") + primary_group_name;
+                int rc = zusf_chown_uss_file_or_dir(&zusf, file_path, owner_str, false);
+                Expect(rc).ToBe(RTNCD_SUCCESS);
+
+                struct stat st{};
+                Expect(stat(file_path.c_str(), &st)).ToBe(0);
+                Expect((int)st.st_gid).ToBe((int)primary_gid);
+
+                unlink(file_path.c_str());
+              });
+
+            it("should chown group-only recursively over a directory tree",
+              [&]() -> void
+              {
+                if (!primary_group_name) {
+                  std::cout << "[SKIP] primary group name not available\n";
+                  return;
+                }
+
+                // project_dir/subdir1/subdir2 with three files
+                mkdir(dir_path.c_str(), 0755);
+                mkdir(dir_a.c_str(), 0755);
+                mkdir(dir_b.c_str(), 0755);
+                { std::ofstream f(f_top); f << "first"; }
+                { std::ofstream f(f_mid); f << "second"; }
+                { std::ofstream f(f_bot); f << "third"; }
+
+                std::string owner_str = std::string(":") + primary_group_name;
+                int rc = zusf_chown_uss_file_or_dir(&zusf, dir_path, owner_str, true);
+                Expect(rc).ToBe(RTNCD_SUCCESS);
+
+                // Verify all dirs and files now have primary_gid
+                auto expect_gid = [&](const std::string& p){
+                  struct stat st{}; Expect(stat(p.c_str(), &st)).ToBe(0);
+                  Expect((int)st.st_gid).ToBe((int)primary_gid);
+                };
+                expect_gid(dir_path);
+                expect_gid(dir_a);
+                expect_gid(dir_b);
+                expect_gid(f_top);
+                expect_gid(f_mid);
+                expect_gid(f_bot);
+
+                // Cleanup
+                unlink(f_bot.c_str());
+                unlink(f_mid.c_str());
+                unlink(f_top.c_str());
+                rmdir(dir_b.c_str());
+                rmdir(dir_a.c_str());
+                rmdir(dir_path.c_str());
+              });
+
+            // Final cleanup
+            rmdir(tmp_base.c_str());
+          });
+
   describe("zusf_chmod_uss_file_or_dir tests",
            [&]() -> void
            {

native/c/zusf.cpp

@@ -9,12 +9,21 @@
  *
  */
 
-#ifndef _LARGE_TIME_API
-#define _LARGE_TIME_API
+// z/OS UNIX extensions needed for st_tag in struct stat, etc.
+#ifndef _AE_BIMODAL
+#define _AE_BIMODAL 1
 #endif
 #ifndef _OPEN_SYS_FILE_EXT
 #define _OPEN_SYS_FILE_EXT 1
 #endif
+#ifndef _LARGE_TIME_API
+#define _LARGE_TIME_API
+#endif
+
+#include <limits.h>
+#include <limits>
+#include <climits>
+
 #ifdef ZSHMEM_ENABLE
 #include "zshmem.hpp"
 #endif
@@ -49,6 +58,7 @@
 #include <time.h>
 #include <iomanip>
 #include <sstream>
+#include <errno.h>
 
 using namespace std;
 
@@ -1599,64 +1609,152 @@ short zusf_get_id_from_user_or_group(const string &user_or_group, bool is_user)
   return -1;
 }
 
-int zusf_chown_uss_file_or_dir(ZUSF *zusf, string file, const string &owner, bool recursive)
+/**
+* Helper to convert user string to UID.
+* Accepts empty (→ -1), numeric UID, or name (getpwnam).
+* Returns true if resolved or empty, false if invalid/overflow.
+ */
+static bool resolve_uid_from_str(const std::string& s, uid_t& out) {
+  if (s.empty()) { out = (uid_t)-1; return true; } // not user provided
+  bool digits = s.find_first_not_of("0123456789") == std::string::npos;
+  if (digits) {
+    unsigned long v = strtoul(s.c_str(), nullptr, 10);
+    if (v > std::numeric_limits<uid_t>::max()) return false;
+    out = static_cast<uid_t>(v);
+    return true;
+  }
+  if (passwd* pw = getpwnam(s.c_str())) { out = pw->pw_uid; return true; }
+  return false;
+}
+
+/**
+* Helper to convert group string to GID.
+* Accepts empty (→ -1), numeric GID, or name (getgrnam).
+* Returns true if resolved or empty, false if invalid/overflow.
+ */
+static bool resolve_gid_from_str(const std::string& s, gid_t& out) {
+  if (s.empty()) { out = (gid_t)-1; return true; } // no group provided
+  bool digits = s.find_first_not_of("0123456789") == std::string::npos;
+  if (digits) {
+    unsigned long v = strtoul(s.c_str(), nullptr, 10);
+    if (v > std::numeric_limits<gid_t>::max()) return false;
+    out = static_cast<gid_t>(v);
+    return true;
+  }
+  if (group* gr = getgrnam(s.c_str())) { out = gr->gr_gid; return true; }
+  return false; // invalid group string
+}
+
+/**
+ * Change ownership of a USS file or directory (recursive optional).
+ *
+ * Supports "user", "user:group", ":group", or numeric IDs.
+ * Validates input, avoids silent -1, and returns RTNCD_FAILURE on error.
+ */
+int zusf_chown_uss_file_or_dir(ZUSF *zusf, std::string file, std::string owner, bool recursive)
 {
   struct stat file_stats;
+  // Verify target exists and capture current metadata
   if (stat(file.c_str(), &file_stats) == -1)
   {
     zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "Path '%s' does not exist", file.c_str());
     return RTNCD_FAILURE;
   }
 
+  // Refuse to descend into a directory if caller didn’t request recursion
   if (S_ISDIR(file_stats.st_mode) && !recursive)
   {
     zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "Path '%s' is a folder and recursive is false", file.c_str());
     return RTNCD_FAILURE;
   }
 
-  const auto uid = zusf_get_id_from_user_or_group(owner, true);
-  const auto colon_pos = owner.find_first_of(":");
-  const auto group = colon_pos != std::string::npos ? owner.substr(colon_pos + 1) : std::string();
-  const auto gid = group.empty() ? file_stats.st_gid : zusf_get_id_from_user_or_group(group, false);
-  const auto rc = chown(file.c_str(), uid, gid);
+  // Split owner into user[:group]
+  std::string userPart = owner;
+  std::string groupPart;
+  const auto colon_pos = owner.find(':');
+  if (colon_pos != std::string::npos) {
+    userPart  = owner.substr(0, colon_pos);
+    groupPart = owner.substr(colon_pos + 1);
+  }
 
-  if (0 != rc)
-  {
-    zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "chmod failed for path '%s', errno %d", file.c_str(), errno);
+  uid_t uid;
+  gid_t gid;
+
+  // Resolve user to UID (numeric or name); return error on invalid input
+  if (!resolve_uid_from_str(userPart, uid)) {
+    errno = EINVAL;
+    zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "chown error: invalid user '%s'", userPart.c_str());
     return RTNCD_FAILURE;
   }
 
+  // Resolve group to GID (numeric or name); return error on invalid input
+  if (!resolve_gid_from_str(groupPart, gid)) {
+    errno = EINVAL;
+    zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "chown error: invalid group '%s'", groupPart.c_str());
+    return RTNCD_FAILURE;
+  }
+
+  // If both were empty, refuse (otherwise chown(-1,-1) is a no-op)
+  if (uid == (uid_t)-1 && gid == (gid_t)-1) {
+    errno = EINVAL;
+    zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "chown error: neither user nor group specified");
+    return RTNCD_FAILURE;
+  }
+
+  // Preserve current group explicitly if only user was supplied
+  if (gid == (gid_t)-1) gid = file_stats.st_gid;
+
+  // Attempt chown
+  const auto rc = chown(file.c_str(), uid, gid);
+  if (rc != 0) {
+    zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "chown failed for path '%s', errno %d", file.c_str(), errno);
+    return RTNCD_FAILURE;
+  }
+
+  // Recurse into directories if requested
   if (recursive && S_ISDIR(file_stats.st_mode))
   {
-    DIR *dir;
-    if ((dir = opendir(file.c_str())) == nullptr)
+    DIR *dir = opendir(file.c_str());
+    if (!dir)
     {
       zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "Could not open directory '%s'", file.c_str());
       return RTNCD_FAILURE;
     }
+
     struct dirent *entry;
     while ((entry = readdir(dir)) != nullptr)
     {
       if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0)
       {
-        const string child_path = file[file.length() - 1] == '/' ? file + string((const char *)entry->d_name)
-                                                                 : file + string("/") + string((const char *)entry->d_name);
-        struct stat file_stats;
-        stat(child_path.c_str(), &file_stats);
+        const string child_path =
+          (file.length() > 0 && file[file.length() - 1] == '/') ? file + string(entry->d_name)
+                               : file + string("/") + string(entry->d_name);
 
-        const auto rc = zusf_chown_uss_file_or_dir(zusf, child_path, owner, S_ISDIR(file_stats.st_mode));
-        if (0 != rc)
+        struct stat child_stats;
+        if (stat(child_path.c_str(), &child_stats) == -1)
         {
-          return rc;
+          closedir(dir);
+          zusf->diag.e_msg_len = sprintf(zusf->diag.e_msg, "Path '%s' no longer accessible", child_path.c_str());
+          return RTNCD_FAILURE;
+        }
+
+        // Propagate chown to children, recursing into subdirectories
+        const auto child_rc =
+          zusf_chown_uss_file_or_dir(zusf, child_path, owner, S_ISDIR(child_stats.st_mode));
+        if (child_rc != 0)
+        {
+          closedir(dir);
+          return child_rc;
         }
       }
     }
+    closedir(dir);
   }
 
   return 0;
 }
 
-int zusf_chtag_uss_file_or_dir(ZUSF *zusf, string file, string tag, bool recursive)
+int zusf_chtag_uss_file_or_dir(ZUSF *zusf, std::string file, std::string tag, bool recursive)
 {
   struct stat file_stats;
   if (stat(file.c_str(), &file_stats) == -1)

native/c/zusf.hpp

@@ -38,9 +38,9 @@ int zusf_write_to_uss_file(ZUSF *zusf, const std::string &file, std::string &dat
 int zusf_write_to_uss_file_streamed(ZUSF *zusf, const std::string &file, const std::string &pipe, size_t *content_len);
 int zusf_chmod_uss_file_or_dir(ZUSF *zusf, std::string file, mode_t mode, bool recursive);
 int zusf_delete_uss_item(ZUSF *zusf, std::string file, bool recursive);
-int zusf_chown_uss_file_or_dir(ZUSF *zusf, std::string file, const std::string &owner, bool recursive);
-short zusf_get_id_from_user_or_group(const std::string &user_or_group, bool is_user);
+int zusf_chown_uss_file_or_dir(ZUSF *zusf, std::string file, std::string owner, bool recursive);
 int zusf_chtag_uss_file_or_dir(ZUSF *zusf, std::string file, std::string tag, bool recursive);
+short zusf_get_id_from_user_or_group(const std::string &user_or_group, bool is_user);
 int zusf_get_file_ccsid(ZUSF *zusf, std::string file);
 std::string zusf_get_ccsid_display_name(int ccsid);
 int zusf_get_ccsid_from_display_name(const std::string &display_name);