Skip to content
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

Can't get rid of x-powered-by header #5699

Closed
RezaRahmati opened this issue Nov 14, 2020 · 10 comments
Closed

Can't get rid of x-powered-by header #5699

RezaRahmati opened this issue Nov 14, 2020 · 10 comments
Labels
needs triage This issue has not been looked into

Comments

@RezaRahmati
Copy link

Bug Report

Current behavior

x-powered-by is present to response
image

Input Code

const server: express.Express = express();

const startNestApplication = async (expressInstance: express.Express) => {
	const adapter = new ExpressAdapter(expressInstance);
	const app = await NestFactory.create<NestExpressApplication>(AppModule, adapter, {});
	app.enableCors();
	app.use(helmet());
	app.disable('x-powered-by');
	app.disable('X-Powered-By');
	app.use(helmet.hidePoweredBy());

Expected behavior

x-powered-by be removed

Possible Solution

Environment


Nest version: 6.10

 
For Tooling issues:
- Node version: 14
- Platform:   Windows 

Others:

@RezaRahmati RezaRahmati added the needs triage This issue has not been looked into label Nov 14, 2020
@jmcdo29
Copy link
Member

jmcdo29 commented Nov 14, 2020

Note that applying helmet as global or registering it must come before other calls to app.use() or setup functions that may call app.use()). This is due to the way the underlying platform (i.e., Express or Fastify) works, where the order that middleware/routes are defined matters. If you use middleware like helmet or cors after you define a route, then that middleware will not apply to that route, it will only apply to middleware defined after the route.

You need to set helmet before calling app.enableCors()

@RezaRahmati
Copy link
Author

RezaRahmati commented Nov 14, 2020

@jmcdo29
Thanks, I just moved the enableCors after use helmet, still same issue

const startNestApplication = async (expressInstance: express.Express) => {
	const adapter = new ExpressAdapter(expressInstance);
	const app = await NestFactory.create<NestExpressApplication>(AppModule, adapter, {});

	app.use(helmet());
	app.disable('x-powered-by');
	app.disable('X-Powered-By');
	app.use(helmet.hidePoweredBy());
	app.use(helmet.hsts({
		maxAge: 15552000,
		includeSubDomains: false,
	}));

        app.enableCors();

	const options = new DocumentBuilder()
		.setTitle('CMOR')
		.setDescription('CMOR API documentation')
		.setVersion('1.0')
		.addServer('/api')
		// .addServer('/web-scanner-dev/us-central1/api')
		.addApiKey({
			type: 'apiKey',
			name: 'api-key',
		}, 'api-key')
		.build();
	const document = SwaggerModule.createDocument(app, options, {
		include: [ScreenAnalyzerModule],
	});

	await app.init();
	return app;
};

@jmcdo29
Copy link
Member

jmcdo29 commented Nov 14, 2020

I can't reproduce this in a local implementation. Any chance you can provide a minimum reproduction?

@RezaRahmati
Copy link
Author

@jmcdo29 sure, will create and provide
May I ask to share your package.json?

@jmcdo29
Copy link
Member

jmcdo29 commented Nov 15, 2020

"dependencies": {
    "@nestjs/common": "^7.0.0",
    "@nestjs/core": "^7.0.0",
    "@nestjs/platform-express": "^7.0.0",
    "helmet": "^4.2.0",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^6.5.4"
  },
  "devDependencies": {
    "@nestjs/cli": "^7.0.0",
    "@nestjs/schematics": "^7.0.0",
    "@nestjs/testing": "^7.0.0",
    "@types/express": "^4.17.3",
    "@types/jest": "26.0.10",
    "@types/node": "^13.9.1",
    "@types/supertest": "^2.0.8",
    "@typescript-eslint/eslint-plugin": "3.9.1",
    "@typescript-eslint/parser": "3.9.1",
    "eslint": "7.7.0",
    "eslint-config-prettier": "^6.10.0",
    "eslint-plugin-import": "^2.20.1",
    "jest": "26.4.2",
    "prettier": "^1.19.1",
    "supertest": "^4.0.2",
    "ts-jest": "26.2.0",
    "ts-loader": "^6.2.1",
    "ts-node": "9.0.0",
    "tsconfig-paths": "^3.9.0",
    "typescript": "^3.7.4"
  },

@RezaRahmati
Copy link
Author

@jmcdo29 I am using 6.1, that could be an issue, I will try to upgrade to 7

@jmcdo29
Copy link
Member

jmcdo29 commented Nov 15, 2020

Doesn't change anything. Just used a v6 server and saw no X-Powered-By header. Please provide a reproduction and I'll reopen the issue

@jmcdo29 jmcdo29 closed this as completed Nov 15, 2020
@RezaRahmati
Copy link
Author

@jmcdo29 Please see this repo, issue is reproducible

https://github.com/RezaRahmati/reproduce-x-powered-by

to run:

cd server
npm i
npm run serve:dev

to check: GET http://localhost:5000/web-scanner-dev/us-central1/api/ping

@jmcdo29
Copy link
Member

jmcdo29 commented Nov 17, 2020

@RezaRahmati your minimum reproduction relies on having the firebase command installed and the serve:dev command fails on running it. Even if firebase-tools is added, there's still the problem of authentication with firebase. This is a lot of code in the main.ts for a minimum reproduction. It seems that the issue is something with firebase and the express instance. Either way, I can't run what you've provided.

If you can provide a minimum reproduction, I'll be happy to take a look again.

@RezaRahmati
Copy link
Author

@jmcdo29 you are right seems firebase functions is adding the header

firebase/firebase-functions#754
https://github.com/expressjs/express/issues/3926

Thanks for help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

2 participants