From 9ae2ecc8287652b64f5d99f3d55598cca1a917c8 Mon Sep 17 00:00:00 2001 From: steven Date: Sat, 5 Sep 2026 14:21:29 +0800 Subject: [PATCH] Improve Pixel logout cleanup and raw responses --- README.md | 16 ++++++++- lib/feedmob/cli/commands/auth.rb | 20 +++++++++-- lib/feedmob/cli/commands/request.rb | 22 +++++++++++++ lib/feedmob/cli/http/client.rb | 5 ++- test/cli_commands_test.rb | 51 +++++++++++++++++++++++++++++ test/http_client_test.rb | 16 +++++++++ 6 files changed, 126 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 27ec7d7..3361f54 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ fm version fm pixel auth login [--token-stdin] fm pixel auth status fm pixel auth logout -fm pixel request get +fm pixel request get [--raw] fm time-off auth login [--token-stdin] fm time-off auth status fm time-off auth logout @@ -88,6 +88,20 @@ fm pixel request get /api/v1/cli/me absolute URLs and `//host` paths are rejected so tokens can never leak to an unconfigured host. Only GET requests are exposed. +Pixel paths are relative to the configured base URL, which already ends in +`/rails`. When copying a path from the Pixel API documentation, remove its +leading `/rails`. Use `--json` to read the response under `data.response`, or +`--raw` to write the exact response body to stdout (for example, redirect a CSV +export to a file). `--raw` and `--json` cannot be combined. Raw requests still +report API errors and exit nonzero; check the exit status before using an export. +A shell redirect can create or truncate the destination even when the request fails. + +Pixel logout removes locally stored credentials even when remote revocation +fails. Such failures retain a nonzero exit status; JSON error details include +`local_removed` and `remote_revoked`. Environment credentials cannot be removed +by the CLI: unset `FEEDMOB_PIXEL_TOKEN` yourself. If revocation failed due to a +network or permission error, the remote token may remain active. + Time Off journal updates use the existing `request` command. Write the JSON request body to a file, then call the documented upsert endpoint; it always writes to the user represented by the configured Time Off token: diff --git a/lib/feedmob/cli/commands/auth.rb b/lib/feedmob/cli/commands/auth.rb index f68b7d7..3c4bf47 100644 --- a/lib/feedmob/cli/commands/auth.rb +++ b/lib/feedmob/cli/commands/auth.rb @@ -65,8 +65,7 @@ def call(**) end remote_revoked = !service.revoke_path.nil? - revoke_remote_token(service, credential.value) if remote_revoked - local_removed = local_credential_source?(credential.source) && runtime.credentials.delete(service) + local_removed = revoke_and_remove(credential, remote_revoked) payload = { service: service.name, logged_out: local_removed || remote_revoked, @@ -82,6 +81,23 @@ def call(**) private + def revoke_and_remove(credential, remote_revoked) + begin + revoke_remote_token(service, credential.value) if remote_revoked + rescue Error => e + local_removed = remove_local_credential(credential) + raise Error.new( + code: e.code, message: "#{e.message} Local credential removed: #{local_removed}.", + details: (e.details || {}).merge(local_removed:, remote_revoked: false), exit_status: e.exit_status + ) + end + remove_local_credential(credential) + end + + def remove_local_credential(credential) + local_credential_source?(credential.source) && runtime.credentials.delete(service) + end + def revoke_remote_token(service, token) runtime.client(service).request(method: :delete, path: service.revoke_path, token:) end diff --git a/lib/feedmob/cli/commands/request.rb b/lib/feedmob/cli/commands/request.rb index 1d853a6..298df97 100644 --- a/lib/feedmob/cli/commands/request.rb +++ b/lib/feedmob/cli/commands/request.rb @@ -27,6 +27,28 @@ def call(path:, **) class PixelRequestGet < RequestGet desc 'Perform an authenticated GET request against the Pixel API' + option :raw, type: :boolean, default: false, desc: 'Write the response body verbatim (incompatible with --json)' + + def call(path:, raw: false, **) + validate_pixel_request!(path, raw) + return super(path:) unless raw + + credential = credential!(service) + response = runtime.client(service).request(method: :get, path:, token: credential.value, raw: true) + (@out || $stdout).write(response.data) + end + + private + + def validate_pixel_request!(path, raw) + if raw && FeedMob::CLI.json? + raise Error.new(code: 'invalid_input', message: '--raw cannot be combined with --json.') + end + return unless URI.parse(service.base_url).path == '/rails' && path.match?(%r{\A/rails(?:/|\?|$)}) + + raise Error.new(code: 'invalid_path', + message: 'Pixel base URL already includes /rails; use /api/v1/... paths.') + end def service_name = 'pixel' end diff --git a/lib/feedmob/cli/http/client.rb b/lib/feedmob/cli/http/client.rb index 8225725..c767f28 100644 --- a/lib/feedmob/cli/http/client.rb +++ b/lib/feedmob/cli/http/client.rb @@ -22,7 +22,10 @@ def request(method:, path:, token:, **options) data = parse_body(raw.fetch(:body)) raise_api_error!(raw.fetch(:status), data) unless (200..299).cover?(raw.fetch(:status)) - Response.new(status: raw.fetch(:status), headers: raw.fetch(:headers), data:) + Response.new( + status: raw.fetch(:status), headers: raw.fetch(:headers), + data: options[:raw] ? raw.fetch(:body) : data + ) rescue Error raise rescue Timeout::Error, SocketError, SystemCallError, OpenSSL::SSL::SSLError => e diff --git a/test/cli_commands_test.rb b/test/cli_commands_test.rb index 89d4920..1a94629 100644 --- a/test/cli_commands_test.rb +++ b/test/cli_commands_test.rb @@ -379,6 +379,57 @@ def test_missing_credential_uses_the_json_error_envelope ) end + def test_pixel_logout_cleans_local_credentials_when_remote_revocation_fails + %w[invalid_token forbidden network_error].each do |code| + %w[keychain encrypted_file env].each do |source| + credentials = FakeCredentials.new(credential: FeedMob::CLI::Credential.new(value: 'fmpat_sentinel', source:)) + client = Object.new + client.define_singleton_method(:request) do |**| + raise FeedMob::CLI::Error.new(code:, message: 'Revocation failed.') + end + use_runtime(credentials:, clients: { 'pixel' => client }) + + stdout, _, status = run_cli('--json', 'pixel', 'auth', 'logout') + + assert_equal 1, status + assert_equal(source == 'env' ? [] : ['pixel'], credentials.deleted) + assert_equal code, JSON.parse(stdout).dig('error', 'code') + assert_equal(source != 'env', JSON.parse(stdout).dig('error', 'details', 'local_removed')) + assert_equal false, JSON.parse(stdout).dig('error', 'details', 'remote_revoked') + end + end + end + + def test_pixel_raw_export_writes_only_csv_bytes + csv = "index,category\r\n1,sample\r\n" + client = FakeClient.new([FeedMob::CLI::HTTP::Response.new(status: 200, headers: {}, data: csv)]) + use_runtime(credentials: FakeCredentials.new, clients: { 'pixel' => client }) + path = '/api/v1/dashboard_api/categories/sample/records/export?advertiser=sample' + + stdout, stderr, status = run_cli('pixel', 'request', 'get', path, '--raw') + + assert_equal 0, status + assert_empty stderr + assert_equal csv, stdout + assert_equal true, client.requests.first.fetch(:raw) + assert_equal path, client.requests.first.fetch(:path) + end + + def test_pixel_rejects_duplicate_prefix_and_conflicting_output_flags_before_request + client = FakeClient.new + use_runtime(credentials: FakeCredentials.new, clients: { 'pixel' => client }) + [ + ['/rails/api/v1/dashboard_api/advertisers', [], 'invalid_path'], + ['/api/v1/dashboard_api/advertisers', %w[--raw --json], 'invalid_input'] + ].each do |path, flags, code| + stdout, stderr, status = run_cli('--json', 'pixel', 'request', 'get', path, *flags) + assert_equal 1, status + assert_empty stderr + assert_equal code, JSON.parse(stdout).dig('error', 'code') + end + assert_empty client.requests + end + private def use_runtime(credentials:, clients:) diff --git a/test/http_client_test.rb b/test/http_client_test.rb index a68ed65..76e3a8f 100644 --- a/test/http_client_test.rb +++ b/test/http_client_test.rb @@ -46,6 +46,22 @@ def test_get_uses_configured_host_bearer_header_and_parses_json ) end + def test_raw_preserves_body_bytes_and_still_raises_api_errors + body = "{ \"advertisers\": [] }\n" + transport = FakeTransport.new(response: { status: 200, headers: {}, body: }) + client = FeedMob::CLI::HTTP::Client.new(service: @service, transport:) + assert_equal body, client.request( + method: :get, path: '/api/v1/dashboard_api/advertisers', token: 'fmpat_sentinel', raw: true + ).data + + transport = FakeTransport.new(response: { status: 401, headers: {}, body: '{"error":{"code":"invalid_token"}}' }) + client = FeedMob::CLI::HTTP::Client.new(service: @service, transport:) + error = assert_raises(FeedMob::CLI::Error) do + client.request(method: :get, path: '/api/v1/dashboard_api/advertisers', token: 'fmpat_sentinel', raw: true) + end + assert_equal 'invalid_token', error.code + end + def test_non_json_response_is_preserved_as_text transport = FakeTransport.new(response: { status: 200, headers: {}, body: 'ok' }) client = FeedMob::CLI::HTTP::Client.new(service: @service, transport:)