Skip to content

Commit 708b72d

Browse files
committed
Use secure temp directories in tests
- Add SecureTempDir helper class using mkdtemp for security - Create temp directories under build/tests instead of /tmp - Auto-cleanup temp directories when tests complete - Update SetGetConfiguredCacheHome test - Update StringConfigFromEnv test - Update MonitoringFileOutput integration test - Update ConcurrentNewIssuerLookup integration test - Update StressTestValidToken integration test - Update StressTestInvalidIssuer integration test This addresses security concerns with predictable /tmp paths and ensures proper cleanup of test artifacts.
1 parent 69e18aa commit 708b72d

File tree

2 files changed

+221
-46
lines changed

2 files changed

+221
-46
lines changed

test/integration_test.cpp

Lines changed: 117 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
#include "../src/scitokens.h"
22

33
#include <atomic>
4+
#include <climits>
45
#include <cmath>
6+
#include <cstdlib>
57
#include <fstream>
68
#include <gtest/gtest.h>
79
#include <iostream>
810
#include <map>
911
#include <memory>
1012
#include <sstream>
1113
#include <string>
14+
#include <sys/stat.h>
1215
#include <thread>
1316
#include <unistd.h>
1417
#include <vector>
@@ -20,6 +23,88 @@
2023

2124
namespace {
2225

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;
70+
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+
};
107+
23108
// Helper class to parse monitoring JSON
24109
class MonitoringStats {
25110
public:
@@ -1037,15 +1122,15 @@ TEST_F(IntegrationTest, MonitoringDurationTracking) {
10371122
TEST_F(IntegrationTest, MonitoringFileOutput) {
10381123
char *err_msg = nullptr;
10391124

1125+
// Create a secure temp directory for the monitoring file
1126+
SecureTempDir temp_dir("monitoring_test_");
1127+
ASSERT_TRUE(temp_dir.valid()) << "Failed to create temp directory";
1128+
10401129
// Set up a test file path and zero interval for immediate write
1041-
std::string test_file = "/tmp/scitokens_monitoring_integration_" +
1042-
std::to_string(time(nullptr)) + ".json";
1130+
std::string test_file = temp_dir.path() + "/monitoring.json";
10431131
scitoken_config_set_str("monitoring.file", test_file.c_str(), &err_msg);
10441132
scitoken_config_set_int("monitoring.file_interval_s", 0, &err_msg);
10451133

1046-
// Clean up any existing file
1047-
std::remove(test_file.c_str());
1048-
10491134
// Reset monitoring stats
10501135
scitoken_reset_monitoring_stats(&err_msg);
10511136

@@ -1119,10 +1204,10 @@ TEST_F(IntegrationTest, MonitoringFileOutput) {
11191204
std::cout << content << std::endl;
11201205
}
11211206

1122-
// Clean up
1207+
// Clean up - disable monitoring file
11231208
scitoken_config_set_str("monitoring.file", "", &err_msg);
11241209
scitoken_config_set_int("monitoring.file_interval_s", 60, &err_msg);
1125-
std::remove(test_file.c_str());
1210+
// temp_dir destructor will clean up the directory and file
11261211
}
11271212

11281213
// =============================================================================
@@ -1347,13 +1432,14 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
13471432
TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
13481433
char *err_msg = nullptr;
13491434

1350-
// Use a unique cache directory to ensure no cached keys exist
1435+
// Use a unique secure cache directory to ensure no cached keys exist
13511436
// This forces the code path where keys must be fetched from the server
1352-
std::string unique_cache_dir = "/tmp/scitokens_concurrent_test_" +
1353-
std::to_string(time(nullptr)) + "_" +
1354-
std::to_string(getpid());
1437+
SecureTempDir unique_cache("concurrent_test_");
1438+
ASSERT_TRUE(unique_cache.valid())
1439+
<< "Failed to create temp cache directory";
1440+
13551441
int rv = scitoken_config_set_str("keycache.cache_home",
1356-
unique_cache_dir.c_str(), &err_msg);
1442+
unique_cache.path().c_str(), &err_msg);
13571443
ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
13581444
<< (err_msg ? err_msg : "unknown");
13591445
if (err_msg) {
@@ -1413,7 +1499,7 @@ TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
14131499
auto initial_key_lookups =
14141500
stats_before.getIssuerStats(issuer_url_).successful_key_lookups;
14151501

1416-
std::cout << "Using unique cache directory: " << unique_cache_dir
1502+
std::cout << "Using unique cache directory: " << unique_cache.path()
14171503
<< std::endl;
14181504
std::cout << "Initial successful_validations: "
14191505
<< initial_successful_validations << std::endl;
@@ -1504,24 +1590,23 @@ TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
15041590
EXPECT_EQ(new_expired_keys, static_cast<uint64_t>(NUM_THREADS))
15051591
<< "All threads should enter the expired_keys code path";
15061592

1507-
// Cleanup: remove the temporary cache directory
1508-
std::string rm_cmd = "rm -rf " + unique_cache_dir;
1509-
(void)system(rm_cmd.c_str());
1593+
// unique_cache destructor will clean up the temporary cache directory
15101594
}
15111595

15121596
// Stress test: repeatedly deserialize a valid token across multiple threads
15131597
// for a fixed duration and verify monitoring counters match actual counts
15141598
TEST_F(IntegrationTest, StressTestValidToken) {
15151599
char *err_msg = nullptr;
15161600

1517-
// Use a unique cache directory to ensure no cached keys exist from prior
1518-
// tests This forces fresh key lookup and prevents background refresh
1519-
// interference
1520-
std::string unique_cache_dir = "/tmp/scitokens_stress_valid_" +
1521-
std::to_string(time(nullptr)) + "_" +
1522-
std::to_string(getpid());
1601+
// Use a unique secure cache directory to ensure no cached keys exist from
1602+
// prior tests. This forces fresh key lookup and prevents background refresh
1603+
// interference.
1604+
SecureTempDir unique_cache("stress_valid_");
1605+
ASSERT_TRUE(unique_cache.valid())
1606+
<< "Failed to create temp cache directory";
1607+
15231608
int rv = scitoken_config_set_str("keycache.cache_home",
1524-
unique_cache_dir.c_str(), &err_msg);
1609+
unique_cache.path().c_str(), &err_msg);
15251610
ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
15261611
<< (err_msg ? err_msg : "unknown");
15271612
if (err_msg) {
@@ -1564,7 +1649,6 @@ TEST_F(IntegrationTest, StressTestValidToken) {
15641649
free(err_msg);
15651650
err_msg = nullptr;
15661651
}
1567-
15681652
std::unique_ptr<void, decltype(&scitoken_destroy)> token(
15691653
scitoken_create(key.get()), scitoken_destroy);
15701654
ASSERT_TRUE(token.get() != nullptr);
@@ -1697,9 +1781,7 @@ TEST_F(IntegrationTest, StressTestValidToken) {
16971781
<< "Should have completed at least 100 validations in "
16981782
<< TEST_DURATION_MS << "ms";
16991783

1700-
// Cleanup: remove the temporary cache directory
1701-
std::string rm_cmd = "rm -rf " + unique_cache_dir;
1702-
(void)system(rm_cmd.c_str());
1784+
// unique_cache destructor will clean up the temporary cache directory
17031785
}
17041786

17051787
// Stress test: repeatedly deserialize a token with an invalid issuer (404)
@@ -1708,13 +1790,14 @@ TEST_F(IntegrationTest, StressTestValidToken) {
17081790
TEST_F(IntegrationTest, StressTestInvalidIssuer) {
17091791
char *err_msg = nullptr;
17101792

1711-
// Use a unique cache directory to ensure no cached keys exist from prior
1712-
// tests
1713-
std::string unique_cache_dir = "/tmp/scitokens_stress_invalid_" +
1714-
std::to_string(time(nullptr)) + "_" +
1715-
std::to_string(getpid());
1793+
// Use a unique secure cache directory to ensure no cached keys exist from
1794+
// prior tests
1795+
SecureTempDir unique_cache("stress_invalid_");
1796+
ASSERT_TRUE(unique_cache.valid())
1797+
<< "Failed to create temp cache directory";
1798+
17161799
int rv = scitoken_config_set_str("keycache.cache_home",
1717-
unique_cache_dir.c_str(), &err_msg);
1800+
unique_cache.path().c_str(), &err_msg);
17181801
ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
17191802
<< (err_msg ? err_msg : "unknown");
17201803
if (err_msg) {
@@ -1873,9 +1956,7 @@ TEST_F(IntegrationTest, StressTestInvalidIssuer) {
18731956
<< "Should have completed at least 100 validations in "
18741957
<< TEST_DURATION_MS << "ms";
18751958

1876-
// Cleanup: remove the temporary cache directory
1877-
std::string rm_cmd = "rm -rf " + unique_cache_dir;
1878-
(void)system(rm_cmd.c_str());
1959+
// unique_cache destructor will clean up the temporary cache directory
18791960
}
18801961

18811962
} // namespace

0 commit comments

Comments
 (0)