Hapi: Improved injected request detection

[neenhouse]

Support plan

• is this issue currently blocking your project? (yes/no): no
• is this issue affecting a production system? (yes/no): no

Context

• node version: any
• module version: >= 4.0.2
• environment (e.g. node, browser, native): node
• used with (e.g. hapi application, another framework, standalone, ...): hapi
• any other relevant information:

What problem are you trying to solve?

We have a use case where we have designed a handler that needs to determine if a request has been injected by @hapi/shot to bypass trusted context validation. Consider the following code:

function skipValidationCheck(req) {
    // inspect request to determine if injected by server, in which case it is already trusted.
    const isRequestInjected = require('@hapi/shot').isInjection(request.raw.req);
    return isRequestInjected;
}

This approach works when a single version of @hapi/shot is installed locally because the underlying symbol on the request object prototype will match the one from the singleton method exported from @hapi/shot. This however breaks down when multiple versions of @hapi/shot are installed due to exported symbols mismatching.

Do you have a new or modified API suggestion to solve the problem?

Proposal is to do one of the following:

• Modify Request.prototype to add isInjected property boolean to consistently identify injected requests.
• or, modify Symbols to export less unique value

I would probably lean to the first bullet as it maintains uniqueness check of symbol, but I don't fully understand the requirements of that. Second bullet would be a simpler approach.

It might also help to modify isInjected documentation to clarify returns true only when versions of module match, unless that was not the intended behavior.

[Marsup]

The root problem is that you have 2 versions of shot, why is that?

[neenhouse]

The situation where this occurs is:

• Team A maintains a hapiJS server
• Team B maintains a hapiJS plugin

Team B maintains an encapsulated feature (security check) that has a piece of logic needs injection detection logic.

The problem is that Team B does not have control over how Team A manages their package.json versions. Team B is responsible for making sure their feature works 100% of the time. Team A installs Team's B package in a way that results in multiple versions of Shot being installed. I don't have a reproducible case, but this will happen when the Team A's version of Shot does not satisfy the required version that Team B declares, and NPM installs sub node_modules directories.

As a work around, Team B has written the following code to ensure they can detect injection:

function isRequestInjected(request) {
    const proto = Object.getPrototypeOf(request);
    const symbols = Object.getOwnPropertySymbols(proto);
    return !!symbols.find(
        (s) => s.description === 'injection' || s.toString() === 'Symbol(injection)'
    );
}

[hueniverse]

The server knows when a request was injected. If this is a teal use case the solution is to expose that information by the server.

[neenhouse]

The server knows when a request was injected.

The specific issue is the team that manages the server code that knows the request is injected is separate from the team that manages the code of the plugin. The server team could expose a piece of information that request as injected, but thought it would be more first class to simply use the underlying abstraction (Shot) to identify itself as injected versus coming up with a convention / contract between Team A and Team B to know how to inspect a request object to see if it's a injected request type.

[hueniverse]

My point is that hapi itself could provide that information.

[neenhouse]

Got it, that would be excellent if the server framework itself was aware of a request being injected. I'm not clear yet on what the proposed implementation would be, thinking through that...

[neenhouse]

Alright, so proposal might look like adding a new method on the Server class prototype that looks something like this:

internals.Server = class {
...
    isRequestInjected(request) {
        return Shot.isInjection(request);
    }
...
}

This would guarantee unique symbol reference between Shot's creating of the Request instance and the check would be fully consistent in any scenario.

@hueniverse if you think this is a sound approach, I'll close my PR here and open a new ticket / PR in the other repo.

[devinivy]

We could also just add this as a property on request objects: request.isInjected, as I think you were recommending in your original approach. We already do something similar on injected auth credentials. I am going to move this issue over to the main hapi repo for further discussion, since it sounds like we generally agree that's where this feature should live if it's going to be implemented.

[neenhouse]

Created an initial proposal to implement @devinivy suggestion to simply add isInjected property.