Skip to content

Commit d14923a

Browse files
bbockelmbbockelm
committed
Refactor SecureTempDir to shared test utility
- Extract SecureTempDir class to test/test_utils.h - Remove duplicated code from main.cpp and integration_test.cpp - Use fork/execv for safe directory removal (prevents shell injection) - Add documentation comments to the utility class
1 parent 708b72d commit d14923a

File tree

3 files changed

+135
-166
lines changed

3 files changed

+135
-166
lines changed

‎test/integration_test.cpp‎

Lines changed: 3 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include "../src/scitokens.h"
2+
#include "test_utils.h"
23

34
#include <atomic>
4-
#include <climits>
55
#include <cmath>
66
#include <cstdlib>
77
#include <fstream>
@@ -21,89 +21,9 @@
2121
#endif
2222
#include <picojson/picojson.h>
2323

24-
namespace {
25-
26-
// Helper class to create and manage secure temporary directories
27-
// Uses mkdtemp for security and cleans up on destruction
28-
class SecureTempDir {
29-
public:
30-
// Create a temp directory under the specified base path
31-
// If base_path is empty, uses BINARY_DIR/tests or falls back to cwd/tests
32-
explicit SecureTempDir(const std::string &prefix = "scitokens_test_",
33-
const std::string &base_path = "") {
34-
std::string base = base_path;
35-
if (base.empty()) {
36-
// Try to use build/tests directory (set by CMake)
37-
const char *binary_dir = std::getenv("BINARY_DIR");
38-
if (binary_dir) {
39-
base = std::string(binary_dir) + "/tests";
40-
} else {
41-
// Fallback: use current working directory + tests
42-
char cwd[PATH_MAX];
43-
if (getcwd(cwd, sizeof(cwd))) {
44-
base = std::string(cwd) + "/tests";
45-
} else {
46-
base = "/tmp"; // Last resort fallback
47-
}
48-
}
49-
}
50-
51-
// Ensure base directory exists
52-
mkdir(base.c_str(), 0700);
53-
54-
// Create template for mkdtemp
55-
std::string tmpl = base + "/" + prefix + "XXXXXX";
56-
std::vector<char> tmpl_buf(tmpl.begin(), tmpl.end());
57-
tmpl_buf.push_back('\0');
58-
59-
char *result = mkdtemp(tmpl_buf.data());
60-
if (result) {
61-
path_ = result;
62-
}
63-
}
64-
65-
~SecureTempDir() { cleanup(); }
66-
67-
// Delete copy constructor and assignment
68-
SecureTempDir(const SecureTempDir &) = delete;
69-
SecureTempDir &operator=(const SecureTempDir &) = delete;
24+
using scitokens_test::SecureTempDir;
7025

71-
// Allow move
72-
SecureTempDir(SecureTempDir &&other) noexcept
73-
: path_(std::move(other.path_)) {
74-
other.path_.clear();
75-
}
76-
77-
SecureTempDir &operator=(SecureTempDir &&other) noexcept {
78-
if (this != &other) {
79-
cleanup();
80-
path_ = std::move(other.path_);
81-
other.path_.clear();
82-
}
83-
return *this;
84-
}
85-
86-
const std::string &path() const { return path_; }
87-
bool valid() const { return !path_.empty(); }
88-
89-
// Manually trigger cleanup
90-
void cleanup() {
91-
if (!path_.empty()) {
92-
// Remove directory contents recursively
93-
remove_directory_recursive(path_);
94-
path_.clear();
95-
}
96-
}
97-
98-
private:
99-
std::string path_;
100-
101-
static void remove_directory_recursive(const std::string &path) {
102-
// Use system rm -rf for simplicity and reliability
103-
std::string cmd = "rm -rf '" + path + "' 2>/dev/null";
104-
(void)system(cmd.c_str());
105-
}
106-
};
26+
namespace {
10727

10828
// Helper class to parse monitoring JSON
10929
class MonitoringStats {

‎test/main.cpp‎

Lines changed: 3 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,96 +1,16 @@
11
#include "../src/scitokens.h"
2+
#include "test_utils.h"
23

3-
#include <climits>
44
#include <cstdlib>
55
#include <cstring>
66
#include <gtest/gtest.h>
77
#include <memory>
88
#include <sys/stat.h>
99
#include <unistd.h>
1010

11-
namespace {
12-
13-
// Helper class to create and manage secure temporary directories
14-
// Uses mkdtemp for security and cleans up on destruction
15-
class SecureTempDir {
16-
public:
17-
// Create a temp directory under the specified base path
18-
// If base_path is empty, uses build/tests or falls back to system temp
19-
explicit SecureTempDir(const std::string &prefix = "scitokens_test_",
20-
const std::string &base_path = "") {
21-
std::string base = base_path;
22-
if (base.empty()) {
23-
// Try to use build/tests directory (set by CMake)
24-
const char *binary_dir = std::getenv("BINARY_DIR");
25-
if (binary_dir) {
26-
base = std::string(binary_dir) + "/tests";
27-
} else {
28-
// Fallback: use current working directory + tests
29-
char cwd[PATH_MAX];
30-
if (getcwd(cwd, sizeof(cwd))) {
31-
base = std::string(cwd) + "/tests";
32-
} else {
33-
base = "/tmp"; // Last resort fallback
34-
}
35-
}
36-
}
37-
38-
// Ensure base directory exists
39-
mkdir(base.c_str(), 0700);
40-
41-
// Create template for mkdtemp
42-
std::string tmpl = base + "/" + prefix + "XXXXXX";
43-
std::vector<char> tmpl_buf(tmpl.begin(), tmpl.end());
44-
tmpl_buf.push_back('\0');
45-
46-
char *result = mkdtemp(tmpl_buf.data());
47-
if (result) {
48-
path_ = result;
49-
}
50-
}
51-
52-
~SecureTempDir() { cleanup(); }
53-
54-
// Delete copy constructor and assignment
55-
SecureTempDir(const SecureTempDir &) = delete;
56-
SecureTempDir &operator=(const SecureTempDir &) = delete;
57-
58-
// Allow move
59-
SecureTempDir(SecureTempDir &&other) noexcept
60-
: path_(std::move(other.path_)) {
61-
other.path_.clear();
62-
}
11+
using scitokens_test::SecureTempDir;
6312

64-
SecureTempDir &operator=(SecureTempDir &&other) noexcept {
65-
if (this != &other) {
66-
cleanup();
67-
path_ = std::move(other.path_);
68-
other.path_.clear();
69-
}
70-
return *this;
71-
}
72-
73-
const std::string &path() const { return path_; }
74-
bool valid() const { return !path_.empty(); }
75-
76-
// Manually trigger cleanup
77-
void cleanup() {
78-
if (!path_.empty()) {
79-
// Remove directory contents recursively
80-
remove_directory_recursive(path_);
81-
path_.clear();
82-
}
83-
}
84-
85-
private:
86-
std::string path_;
87-
88-
static void remove_directory_recursive(const std::string &path) {
89-
// Use system rm -rf for simplicity and reliability
90-
std::string cmd = "rm -rf '" + path + "' 2>/dev/null";
91-
(void)system(cmd.c_str());
92-
}
93-
};
13+
namespace {
9414

9515
const char ec_private[] =
9616
"-----BEGIN EC PRIVATE KEY-----\n"

‎test/test_utils.h‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#ifndef SCITOKENS_TEST_UTILS_H
2+
#define SCITOKENS_TEST_UTILS_H
3+
4+
#include <climits>
5+
#include <cstdlib>
6+
#include <string>
7+
#include <sys/stat.h>
8+
#include <sys/wait.h>
9+
#include <unistd.h>
10+
#include <vector>
11+
12+
namespace scitokens_test {
13+
14+
/**
15+
* Helper class to create and manage secure temporary directories.
16+
* Uses mkdtemp for security and cleans up on destruction.
17+
*
18+
* Example usage:
19+
* SecureTempDir temp_dir("my_test_");
20+
* ASSERT_TRUE(temp_dir.valid());
21+
* std::string cache_path = temp_dir.path() + "/cache";
22+
* // ... use the directory ...
23+
* // Directory is automatically cleaned up when temp_dir goes out of scope
24+
*/
25+
class SecureTempDir {
26+
public:
27+
/**
28+
* Create a temp directory under the specified base path.
29+
* @param prefix Prefix for the directory name (default: "scitokens_test_")
30+
* @param base_path Base path for the temp directory. If empty, uses
31+
* BINARY_DIR/tests (from CMake) or falls back to cwd/tests
32+
*/
33+
explicit SecureTempDir(const std::string &prefix = "scitokens_test_",
34+
const std::string &base_path = "") {
35+
std::string base = base_path;
36+
if (base.empty()) {
37+
// Try to use build/tests directory (set by CMake)
38+
const char *binary_dir = std::getenv("BINARY_DIR");
39+
if (binary_dir) {
40+
base = std::string(binary_dir) + "/tests";
41+
} else {
42+
// Fallback: use current working directory + tests
43+
char cwd[PATH_MAX];
44+
if (getcwd(cwd, sizeof(cwd))) {
45+
base = std::string(cwd) + "/tests";
46+
} else {
47+
base = "/tmp"; // Last resort fallback
48+
}
49+
}
50+
}
51+
52+
// Ensure base directory exists
53+
mkdir(base.c_str(), 0700);
54+
55+
// Create template for mkdtemp
56+
std::string tmpl = base + "/" + prefix + "XXXXXX";
57+
std::vector<char> tmpl_buf(tmpl.begin(), tmpl.end());
58+
tmpl_buf.push_back('\0');
59+
60+
char *result = mkdtemp(tmpl_buf.data());
61+
if (result) {
62+
path_ = result;
63+
}
64+
}
65+
66+
~SecureTempDir() { cleanup(); }
67+
68+
// Delete copy constructor and assignment
69+
SecureTempDir(const SecureTempDir &) = delete;
70+
SecureTempDir &operator=(const SecureTempDir &) = delete;
71+
72+
// Allow move
73+
SecureTempDir(SecureTempDir &&other) noexcept
74+
: path_(std::move(other.path_)) {
75+
other.path_.clear();
76+
}
77+
78+
SecureTempDir &operator=(SecureTempDir &&other) noexcept {
79+
if (this != &other) {
80+
cleanup();
81+
path_ = std::move(other.path_);
82+
other.path_.clear();
83+
}
84+
return *this;
85+
}
86+
87+
/** Get the path to the temporary directory */
88+
const std::string &path() const { return path_; }
89+
90+
/** Check if the directory was created successfully */
91+
bool valid() const { return !path_.empty(); }
92+
93+
/** Manually trigger cleanup (also called by destructor) */
94+
void cleanup() {
95+
if (!path_.empty()) {
96+
remove_directory_recursive(path_);
97+
path_.clear();
98+
}
99+
}
100+
101+
private:
102+
std::string path_;
103+
104+
/**
105+
* Safely remove a directory recursively using fork/execv.
106+
* This prevents shell injection attacks that could occur with system().
107+
*/
108+
static void remove_directory_recursive(const std::string &path) {
109+
pid_t pid = fork();
110+
if (pid == 0) {
111+
// Child process: exec rm -rf with path as direct argument
112+
// Using execv prevents any shell interpretation of the path
113+
char *const args[] = {const_cast<char *>("rm"),
114+
const_cast<char *>("-rf"),
115+
const_cast<char *>(path.c_str()), nullptr};
116+
execv("/bin/rm", args);
117+
_exit(1); // execv failed
118+
} else if (pid > 0) {
119+
// Parent: wait for child to complete
120+
int status;
121+
waitpid(pid, &status, 0);
122+
}
123+
// If fork failed, silently ignore (cleanup is best-effort)
124+
}
125+
};
126+
127+
} // namespace scitokens_test
128+
129+
#endif // SCITOKENS_TEST_UTILS_H

0 commit comments

Comments
 (0)