From 5d3912962b8da28c377eb9dbfac022f1bffc80d7 Mon Sep 17 00:00:00 2001
From: Cole Helbling <cole.helbling@determinate.systems>
Date: Mon, 24 Jan 2022 11:25:13 -0800
Subject: [PATCH] RunCommand: use helper functions to ensure filenames and
 paths are the same

Otherwise, it's possible someone updates the format in one place but not
the others, leading to broken or incorrect functionality.
---
 src/lib/Hydra/Controller/Build.pm  |  2 +-
 src/lib/Hydra/Controller/Root.pm   |  3 +--
 src/lib/Hydra/Helper/Nix.pm        | 18 ++++++++++++++++++
 src/lib/Hydra/Plugin/RunCommand.pm | 16 +++++++++-------
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm
index a833ae04..9a2c5607 100644
--- a/src/lib/Hydra/Controller/Build.pm
+++ b/src/lib/Hydra/Controller/Build.pm
@@ -140,7 +140,7 @@ sub view_log : Chained('buildChain') PathPart('log') {
 sub view_runcommand_log : Chained('buildChain') PathPart('runcommand-log') {
     my ($self, $c, $sha) = @_;
 
-    $c->stash->{log_uri} = $c->uri_for($c->controller('Root')->action_for("runcommandlog"), $sha . "-" . $c->stash->{build}->id);
+    $c->stash->{log_uri} = $c->uri_for($c->controller('Root')->action_for("runcommandlog"), constructRunCommandLogFilename($sha, $c->stash->{build}->id));
     $c->stash->{template} = 'runcommand-log.tt';
 }
 
diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm
index abb95c7c..81f79378 100644
--- a/src/lib/Hydra/Controller/Root.pm
+++ b/src/lib/Hydra/Controller/Root.pm
@@ -7,7 +7,6 @@ use base 'Hydra::Base::Controller::ListBuilds';
 use Hydra::Helper::Nix;
 use Hydra::Helper::CatalystUtils;
 use Hydra::View::TT;
-use Hydra::Model::DB;
 use Nix::Store;
 use Nix::Config;
 use Encode;
@@ -538,7 +537,7 @@ sub runcommandlog :Local :Args(1) {
 
     die if defined $tail && $tail !~ /^[0-9]+$/;
 
-    my $logFile = Hydra::Model::DB::getHydraPath . "/runcommand-logs/" . substr($filename, 0, 2) . "/$filename";
+    my $logFile = constructRunCommandLogPath($filename);
     if (-f $logFile) {
         serveLogFile($c, $logFile, $tail);
         return;
diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm
index 55c1f6f3..a58e638d 100644
--- a/src/lib/Hydra/Helper/Nix.pm
+++ b/src/lib/Hydra/Helper/Nix.pm
@@ -19,6 +19,8 @@ our @EXPORT = qw(
     cancelBuilds
     captureStdoutStderr
     captureStdoutStderrWithStdin
+    constructRunCommandLogFilename
+    constructRunCommandLogPath
     findLog
     gcRootFor
     getBaseUrl
@@ -589,4 +591,20 @@ sub isLocalStore {
 }
 
 
+sub constructRunCommandLogFilename {
+    my ($sha, $build_id) = @_;
+    my $filename = "$sha-$build_id";
+    return $filename;
+}
+
+
+sub constructRunCommandLogPath {
+    my ($filename) = @_;
+
+    my $hydra_path = Hydra::Model::DB::getHydraPath;
+    my $bucket = substr($filename, 0, 2);
+
+    return "$hydra_path/runcommand-logs/$bucket/$filename";
+}
+
 1;
diff --git a/src/lib/Hydra/Plugin/RunCommand.pm b/src/lib/Hydra/Plugin/RunCommand.pm
index 096bf1c1..725f7aaa 100644
--- a/src/lib/Hydra/Plugin/RunCommand.pm
+++ b/src/lib/Hydra/Plugin/RunCommand.pm
@@ -7,6 +7,8 @@ use experimental 'smartmatch';
 use JSON::MaybeXS;
 use Digest::SHA1 qw(sha1_hex);
 use Hydra::Model::DB;
+use Hydra::Helper::Nix;
+use File::Basename qw(dirname);
 
 sub isEnabled {
     my ($self) = @_;
@@ -162,15 +164,15 @@ sub buildFinished {
 
         $runlog->started();
 
-        # Prepare log collection
-        my $filename = sha1_hex($command) . "-" . $build->get_column('id');
-        my $dir = Hydra::Model::DB::getHydraPath . "/runcommand-logs/" . substr($filename, 0, 2);
-        my $logpath = "$dir/$filename";
+        my $filename = constructRunCommandLogFilename(sha1_hex($command), $build->get_column('id'));
+        my $logPath = constructRunCommandLogPath($filename);
+        my $dir = dirname($logPath);
+
         mkdir($dir, oct(755));
-        # This creates the file with the correct permissions
-        open(my $f, '>', $logpath);
+
+        open(my $f, '>', $logPath);
         close($f);
-        chmod(oct(644), $logpath);
+        chmod(oct(644), $logPath);
 
         # Run the command
         system("$command 1>$logpath 2>&1") == 0