Skip to content

Log HTTP requests/responses (only non-sensitive data) #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions bin/console.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
use Laminas\AutomaticReleases\Github\JwageGenerateChangelog;
use Laminas\AutomaticReleases\Github\MergeMultipleReleaseNotes;
use Laminas\AutomaticReleases\Gpg\ImportGpgKeyFromStringViaTemporaryFile;
use Laminas\AutomaticReleases\HttpClient\LoggingHttpClient;
use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings;
use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpResponsesIntoStrings;
use Lcobucci\Clock\SystemClock;
use Monolog\Handler\StreamHandler;
use Monolog\Logger;
Expand All @@ -60,14 +63,20 @@ static function (int $errorCode, string $message = '', string $file = '', int $l
E_STRICT | E_NOTICE | E_WARNING,
);

$variables = EnvironmentVariables::fromEnvironment(new ImportGpgKeyFromStringViaTemporaryFile());
$logger = new Logger('automatic-releases');
$logger->pushHandler(new StreamHandler(STDERR, $variables->logLevel()));
$variables = EnvironmentVariables::fromEnvironment(new ImportGpgKeyFromStringViaTemporaryFile());
$logger = new Logger(
'automatic-releases',
[new StreamHandler(STDERR, $variables->logLevel())],
[
new ConvertLogContextHttpRequestsIntoStrings(),
new ConvertLogContextHttpResponsesIntoStrings(),
],
);
$loadEvent = new LoadCurrentGithubEventFromGithubActionPath($variables);
$fetch = new FetchAndSetCurrentUserByReplacingCurrentOriginRemote($variables);
$getCandidateBranches = new GetMergeTargetCandidateBranchesFromRemoteBranches();
$makeRequests = Psr17FactoryDiscovery::findRequestFactory();
$httpClient = HttpClientDiscovery::find();
$httpClient = new LoggingHttpClient(HttpClientDiscovery::find(), $logger);
$githubToken = $variables->githubToken();
$getMilestone = new GetMilestoneFirst100IssuesAndPullRequests(new RunGraphQLQuery(
$makeRequests,
Expand Down
2 changes: 1 addition & 1 deletion infection.json.dist
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
"mutators": {
"@default": true
},
"minMsi": 97,
"minMsi": 98,
"minCoveredMsi": 100
}
13 changes: 0 additions & 13 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,6 @@
</ignoreFiles>
</projectFiles>

<issueHandlers>
<InternalMethod>
<errorLevel type="suppress">
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::method"/>
</errorLevel>
<errorLevel type="suppress">
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn"/>
</errorLevel>
<errorLevel type="suppress">
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::with"/>
</errorLevel>
</InternalMethod>
</issueHandlers>
<plugins>
<pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
<pluginClass class="Psl\Psalm\Plugin"/>
Expand Down
3 changes: 1 addition & 2 deletions src/Changelog/ChangelogReleaseNotes.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ public function merge(self $next): self
{
if ($this->changelogEntry && $next->changelogEntry) {
throw new RuntimeException(
'Aborting: Both current release notes and next contain a ChangelogEntry;'
. ' only one CreateReleaseText implementation should resolve one.',
'Aborting: Both current release notes and next contain a ChangelogEntry; only one CreateReleaseText implementation should resolve one.',
);
}

Expand Down
10 changes: 7 additions & 3 deletions src/Git/Value/MergeTargetCandidateBranches.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ public static function fromAllBranches(BranchName ...$branches): self
return $branch->isReleaseBranch();
});

$mergeTargetBranches = Vec\sort($mergeTargetBranches, static function (BranchName $a, BranchName $b): int {
return $a->majorAndMinor() <=> $b->majorAndMinor();
});
$mergeTargetBranches = Vec\sort($mergeTargetBranches, self::branchOrder(...));

return new self($mergeTargetBranches);
}
Expand Down Expand Up @@ -98,4 +96,10 @@ public function contains(BranchName $needle): bool
static fn (BranchName $branch): bool => $needle->equals($branch)
);
}

/** @return -1|0|1 */
private static function branchOrder(BranchName $a, BranchName $b): int
{
return $a->majorAndMinor() <=> $b->majorAndMinor();
}
}
19 changes: 11 additions & 8 deletions src/Github/CreateReleaseTextViaKeepAChangelog.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,20 @@ private function updateReleaseDate(string $changelog, string $version): string
*/
private function removeDefaultContents(string $changelog): string
{
$contents = Iter\reduce(
return Type\non_empty_string()->assert(Iter\reduce(
self::DEFAULT_SECTIONS,
static fn (string $changelog, string $section): string => Regex\replace(
$changelog,
"/\n\#{3} " . $section . "\n\n- Nothing.\n/s",
'',
),
self::removeEmptyDefaultChangelogSection(...),
$changelog,
);
));
}

return Type\non_empty_string()->assert($contents);
private static function removeEmptyDefaultChangelogSection(string $changelog, string $section): string
{
return Regex\replace(
$changelog,
"/\n\#{3} " . $section . "\n\n- Nothing.\n/s",
'',
);
}

/**
Expand Down
35 changes: 35 additions & 0 deletions src/HttpClient/LoggingHttpClient.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\HttpClient;

use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Log\LoggerInterface;

/** @internal */
final class LoggingHttpClient implements ClientInterface
{
public function __construct(private readonly ClientInterface $next, private readonly LoggerInterface $logger)
{
}

public function sendRequest(RequestInterface $request): ResponseInterface
{
$this->logger->debug('Sending request {request}', ['request' => $request]);

$response = $this->next->sendRequest($request);

$this->logger->debug(
'Received response {response} to request {request}',
[
'request' => $request,
'response' => $response,
],
);

return $response;
}
}
42 changes: 42 additions & 0 deletions src/Monolog/ConvertLogContextHttpRequestsIntoStrings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\Monolog;

use Monolog\LogRecord;
use Monolog\Processor\ProcessorInterface;
use Psr\Http\Message\RequestInterface;

use function array_map;

/** @internal */
final class ConvertLogContextHttpRequestsIntoStrings implements ProcessorInterface
{
public function __invoke(LogRecord $record): LogRecord
{
return new LogRecord(
$record->datetime,
$record->channel,
$record->level,
$record->message,
array_map(self::contextItemToMessage(...), $record->context),
$record->extra,
$record->formatted,
);
}

private static function contextItemToMessage(mixed $item): mixed
{
if (! $item instanceof RequestInterface) {
return $item;
}

return $item->getMethod()
. ' '
. $item
->getUri()
->withUserInfo('')
->__toString();
}
}
42 changes: 42 additions & 0 deletions src/Monolog/ConvertLogContextHttpResponsesIntoStrings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\Monolog;

use Monolog\LogRecord;
use Monolog\Processor\ProcessorInterface;
use Psr\Http\Message\ResponseInterface;

use function array_map;

/** @internal */
final class ConvertLogContextHttpResponsesIntoStrings implements ProcessorInterface
{
public function __invoke(LogRecord $record): LogRecord
{
return new LogRecord(
$record->datetime,
$record->channel,
$record->level,
$record->message,
array_map(self::contextItemToMessage(...), $record->context),
$record->extra,
$record->formatted,
);
}

private static function contextItemToMessage(mixed $item): mixed
{
if (! $item instanceof ResponseInterface) {
return $item;
}

return $item->getStatusCode()
. ' "'
. $item
->getBody()
->__toString()
. '"';
}
}
51 changes: 51 additions & 0 deletions test/unit/HttpClient/LoggingHttpClientTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\Test\Unit\HttpClient;

use Http\Discovery\Psr17FactoryDiscovery;
use Laminas\AutomaticReleases\HttpClient\LoggingHttpClient;
use PHPUnit\Framework\TestCase;
use Psr\Http\Client\ClientInterface;
use Psr\Log\LoggerInterface;

/** @covers \Laminas\AutomaticReleases\HttpClient\LoggingHttpClient */
final class LoggingHttpClientTest extends TestCase
{
public function testWillLogRequestAndResponse(): void
{
$request = Psr17FactoryDiscovery::findRequestFactory()->createRequest('get', 'http://example.com/foo/bar');
$response = Psr17FactoryDiscovery::findResponseFactory()->createResponse(204);

$response->getBody()
->write('hello world');

$logger = $this->createMock(LoggerInterface::class);
$next = $this->createMock(ClientInterface::class);

$next->expects(self::once())
->method('sendRequest')
->with($request)
->willReturn($response);

$logger->expects(self::exactly(2))
->method('debug')
->withConsecutive(
['Sending request {request}', ['request' => $request]],
[
'Received response {response} to request {request}',
[
'request' => $request,
'response' => $response,
],
],
);

self::assertSame(
$response,
(new LoggingHttpClient($next, $logger))
->sendRequest($request),
);
}
}
56 changes: 56 additions & 0 deletions test/unit/Monolog/ConvertLogContextHttpRequestsIntoStringsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\Test\Unit\Monolog;

use DateTimeImmutable;
use Http\Discovery\Psr17FactoryDiscovery;
use Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings;
use Monolog\Level;
use Monolog\LogRecord;
use PHPUnit\Framework\TestCase;

/** @covers \Laminas\AutomaticReleases\Monolog\ConvertLogContextHttpRequestsIntoStrings */
final class ConvertLogContextHttpRequestsIntoStringsTest extends TestCase
{
public function testWillScrubSensitiveRequestInformation(): void
{
$date = new DateTimeImmutable();

$requestFactory = Psr17FactoryDiscovery::findRequestFactory();

$plainRequest = $requestFactory->createRequest('GET', 'http://example.com/foo');

$sensitiveRequest = $requestFactory->createRequest('POST', 'https://user:[email protected]/foo?bar=baz')
->withAddedHeader('Authentication', ['also secret']);

$sensitiveRequest->getBody()
->write('super: secret');

self::assertEquals(
new LogRecord(
$date,
'a-channel',
Level::Critical,
'a message',
[
'foo' => 'bar',
'plain request' => 'GET http://example.com/foo',
'sensitive request' => 'POST https://example.com/foo?bar=baz',
],
),
(new ConvertLogContextHttpRequestsIntoStrings())(new LogRecord(
$date,
'a-channel',
Level::Critical,
'a message',
[
'foo' => 'bar',
'plain request' => $plainRequest,
'sensitive request' => $sensitiveRequest,
],
)),
);
}
}
Loading